Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

doc: Repository structure and modules #20067

Closed
wants to merge 2 commits into from
Closed
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
96 changes: 70 additions & 26 deletions doc/guides/modules.rst
Original file line number Diff line number Diff line change
Expand Up @@ -26,25 +26,32 @@ build and configure them, respectively. Module :file:`CMakeLists.txt` files are
added to the build using CMake's `add_subdirectory()`_ command, and the
:file:`Kconfig` files are included in the build's Kconfig menu tree.

If you have :ref:`west <west>` installed, you don't need to worry about how
this variable is defined unless you are adding a new module. The build system
knows how to use west to set :makevar:`ZEPHYR_MODULES`. You can add additional
modules to this list by setting the :makevar:`ZEPHYR_EXTRA_MODULES` CMake
variable or by adding a :makevar:`ZEPHYR_EXTRA_MODULES` line to ``.zephyrrc``
(See the section on :ref:`env_vars` for more details). This can be
useful if you want to keep the list of modules found with west and also add
your own.

See the section about :ref:`west-multi-repo` for more details.

Finally, you can also specify the list of modules yourself in various ways, or
not use modules at all if your application doesn't need them.


Each module used by upstream Zephyr is contained in its own Git repository, and
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/upstream/mainline

the set of Git repositories is managed by :ref:`west <west>`. See
:ref:`repo-structure` for more information on the Zephyr repository topology,
and :ref:`west-multi-repo` for more details on how west manages multiple
repositories.

In that way, if you use upstream Zephyr and therefore west, you don't need to
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/upstream/mainline

worry about how this variable is defined at all. The build system knows how to
use west to set :makevar:`ZEPHYR_MODULES`. You can add additional modules to
this list by setting the :makevar:`ZEPHYR_EXTRA_MODULES` CMake variable or by
setting the :makevar:`ZEPHYR_EXTRA_MODULES` environment varialbe in
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

variable

``.zephyrrc`` (See the section on :ref:`env_vars` for more details). This can be
useful if you want to extend the list of modules found with west with your own
local ones without having to modify the `manifest file <west.yml_>`_.

It is also possible to avoid using west or modules at all, see the section below
for more information.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not just

please see :ref:`no-west` for details.

?


Module Inclusion
****************

.. note::
In all cases below the :makevar:`ZEPHYR_EXTRA_MODULES` environment and CMake
variables are taken into account only if the :makevar:`ZEPHYR_MODULES` CMake
variable is set, since the former is an extension to the latter.

.. _modules_using_west:

Using West
Expand Down Expand Up @@ -179,13 +186,15 @@ Zephyr project issue tracking system with details about the module and how it
integrates into the project. If the request is approved, a new repository will
created by the project team and initialized with basic information that would
allow submitting code to the module project following the project contribution
guidelines.
guidelines. The request may be approved by a maintainer of a particular
subsystem or may have to be escalated to the TSC.

All modules should be hosted in repositories under the Zephyr organization. The
manifest should only point to repositories maintained under the Zephyr project.
If a module is maintained as a fork of another project on Github, the Zephyr module
related files and changes in relation to upstream need to be maintained in a
special branch named ``zephyr``.
All modules must be hosted in repositories under the `Zephyr organization
<zephyrproject-rtos_>`_, and thus the manifest must only point to repositories
maintained under the Zephyr project. If a module is maintained as a fork of an
externally maintained project on Github or elsehwere, the Zephyr module related
files and changes in relation to upstream must be committed to the module
repository's ``master`` branch. See :ref:`branches` for additional information.

Process
-------
Expand All @@ -196,11 +205,12 @@ Follow the following steps to request a new module:
created
#. Propose a name for the repository to be created under the Zephyr project
organization on Github.
#. Maintainers from the Zephyr project will create the repository and initialize
it. You will be added as a collaborator in the new repository.
#. If the request is approved, maintainers from the Zephyr project will create
the repository and initialize it. You will be added as a collaborator in the
new repository.
#. Submit the module content (code) to the new repository following the
guidelines described :ref:`here <modules_using_west>`.
#. Add a new entry to the :zephyr_file:`west.yml` with the following
#. Add a new entry to the `west.yml`_ manifest file with the following
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd recommend leaving this as :zephyr_file:`west.yml` (as noted below, you shouldn't lock it to master)

information:

.. code-block:: console
Expand Down Expand Up @@ -231,7 +241,7 @@ Changes to existing modules
#. Submit the changes using a pull request to an existing repository following
the :ref:`contribution guidelines <contribute_guidelines>`.
#. Submit a pull request changing the entry referencing the module into the
:zephyr_file:`west.yml` of the main Zephyr tree with the following
`west.yml`_ of the main Zephyr tree with the following
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd recommend leaving this as :zephyr_file:`west.yml` (as noted below, you shouldn't lock it to master)

information:

.. code-block:: console
Expand All @@ -253,9 +263,43 @@ Where 23 in the example above indicated the pull request number submitted to the
*my_module* repository. Once the module changes are reviewed and merged, the
revision needs to be changed to the commit hash from the module repository.


Upmerging forks
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"upmerging" is an NCS term I would like to avoid in official upstream docs.

"Synchronizing forks" perhaps? Here and elsewhere in this section.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

agree, I do not really know what upmerging means in this context.

===============

When a module is in fact a fork of an existing, external project that is hosted
on GitHub or elsewhere, it is often necessary to upmerge it: update the copy
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

avoid upmerge, maybe synchronize?

that is hosted under the `zephyrproject-rtos`_ GitHub organization with the
latest changes from the external upstream repository.

The procedure for upmerging module repositories that are forks of external
projects is constrained by the following two factors:

* History must **never** be rewritten in any repository that is referenced in
the `west.yml`_ manifest file, since that could break older revisions of the
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why? We could prevent the old commits being garbage collected using tags, as we do downstream.

repository set as a whole. This therefore excludes rebasing the module
repository and keeping the zephyr-specific changes always on top.
* GitHub does not deal correctly with merge commits contained inside Pull
Requests, which prevents us from using regular ``git merge`` operations
without having to resort to pushing directly to the repository.

Due to the above, this is the procedure that you must use in order to upmerge a
module repository with the latest changes from the external upstream
repository:

#. Make a note of the current upstream revision that the module repository is
currently upmerged to.
#. Take the changes from the current to the latest upstream revision and apply
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This might be controversial, but it's the only way I can think of to avoid the usual upmerge pitfalls.
Perhaps we should add a templated commit message here that says:

upmerge: Update from SHA1 to SHA2

Origin: project@SHA2

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The other option is to allow multiple upmerge strategies

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To say publicly what I've said before in slack/telecons: When I'm working on forks like those Zephyr requires I usually use a workspace with access to every upstream source. I find it extremely inconvenient to be guessing what upstream commit a particular fork commit is supposed to be equivalent to. A comment in a commit would at least provide a clue, but allowing real merge commits where you can see the upstream tag in its canonical form associated with the real SHA1 is the only sane way to be doing cross-repository comparisons.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree that real merge commits (if merging) is the sane way to handle it.

I still think we should consider allowing sporadic rebasing, but that's a different question.

them to the current ``master`` branch of the module repository. There are
several ways of achieving this It is important to note that the
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

missing a period at the end of the sentence this. It is

module repository may not share a common Git history with the original
upstream repository, so using ``git diff`` and ``git apply`` might be the
simplest course of action.
#. Submit a pull request against the module and zephyr (manifest) repository as
described in :ref:`changes_to_existing_module`.

.. _CMake list: https://cmake.org/cmake/help/latest/manual/cmake-language.7.html#lists
.. _add_subdirectory(): https://cmake.org/cmake/help/latest/command/add_subdirectory.html

.. _GitHub issues: https://github.com/zephyrproject-rtos/zephyr/issues
.. _zephyrproject-rtos: https://github.com/zephyrproject-rtos
.. _west.yml: https://github.com/zephyrproject-rtos/zephyr/blob/master/west.yml
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are you removing the :zephyr_file: uses?

That custom role has a hope of being extended to not go stale by emitting links for release documentation that don't point at master. This doesn't.

See the discussion here for details on my concern: #19123 (comment)