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

West update tooling dependency management #55056

Open
Tracked by #54276
yperess opened this issue Feb 21, 2023 · 14 comments
Open
Tracked by #54276

West update tooling dependency management #55056

yperess opened this issue Feb 21, 2023 · 14 comments
Assignees
Labels
area: West West utility RFC Request For Comments: want input from the community

Comments

@yperess
Copy link
Collaborator

yperess commented Feb 21, 2023

Introduction

This work intends to automate the tooling dependency management of Zephyr projects. The idea is to script https://docs.zephyrproject.org/latest/develop/getting_started/index.html in an extensible way that will work for most projects.

NOTE: this is not intended to manage zephyr modules. Those will still need to be done the same way they currently are via the West projects.

Problem description

Managing dependencies and communicating changes to these dependencies comes with unnecessary overhead that can easily be automated

Proposed change

  1. Add a dependencies section to module manifests. This YAML entry will be a list of tuples (type, name, version_string).
  2. When we run west update get a snapshot of all the modules + zephyr and take the union of the dependencies
  3. After updating the module repos, take a second snapshot of the dependencies

Milestone 1

As step 4, print the changed dependencies

Milestone 2

Add a new top level west class WestInstaller that's similar to how west commands are done. The west.yml will be able to map dependency type to an installer. By default python type will map to pip, native under linux can be mapped to an installer that calls apt-get etc. This way downstream clients can remap the installers to their own trusted sources or include any custom install instructions they might need. west update will automatically attempt to install the dependencies. This can easily be turned on/off via a west config value (we can default it to false).

Detailed RFC

Proposed change (Detailed)

The first step is to add the dependency list to Zephyr and the official modules. Each repo will include a PR that adds a list of dependencies which currently include the following:

  1. python - a single python package dependency
  2. python-list - a list of python dependencies that will be installed using the -r flag in pip
  3. native - a native executable that needs to be available.

At this point, we'll add a new flag (west update --list-deps) which will allow the developers to see the current snapshot of the dependencies. We'll also add a new west config value which will be defaulted to false to enable dependency management in west update. If enabled by the west.yml of the project, then west update will print the dependency diff at the end.

The second step will be to add the top level WestInstaller class and a PipInstaller and PipListInstaller subclasses that will handle python and python-list types. If the above config value is enabled, we'll also by default create the .venv python virtual environment or read a name for the virtual environment from another config value.

The third step will be to add the NativeInstaller which will handle the native type for various operating systems (automativing https://docs.zephyrproject.org/latest/develop/getting_started/index.html#install-dependencies).

@yperess yperess added the RFC Request For Comments: want input from the community label Feb 21, 2023
@henrikbrixandersen henrikbrixandersen added the area: West West utility label Feb 22, 2023
@nordicjm
Copy link
Collaborator

My view on this is use a package manager if you want a package manager and keep it out of west, the package manager can invoke west commands.

@nashif
Copy link
Member

nashif commented Feb 22, 2023

@yperess there are two levels of dependencies to consider:

  1. arch/board/soc/subsystem require a module A, for example, an STMicro board requires the ST hal
  2. module A requires a binary: For example, nanopb requires protoc or thrift module requires thrift package

Does this RFC cover point 2 only or both?

@yperess
Copy link
Collaborator Author

yperess commented Feb 22, 2023

@yperess there are two levels of dependencies to consider:

  1. arch/board/soc/subsystem require a module A, for example, an STMicro board requires the ST hal
  2. module A requires a binary: For example, nanopb requires protoc or thrift module requires thrift package

Does this RFC cover point 2 only or both?

This proposal covers the second case. I hadn't considered checking if a module is added (I assume at build time since it'll be config/board dependent) but that's an interesting use. We would need to assign name/version to modules but that can easily be done in the module manifest. Since people can have their own version of modules (maybe their own forks out computer custom logic they would just need to make sure it matches the name). Let me think about that some more.

@yperess
Copy link
Collaborator Author

yperess commented Feb 22, 2023

My view on this is use a package manager if you want a package manager and keep it out of west, the package manager can invoke west commands.

I'm not familiar with a package manager that can handle multiple types. Afaik they are usually separated by domains; for example: pip (python), npm (node), APT (Ubuntu binaries). Are you familiar with one that manages all? The issue will remain that downstream might need to override the source of these. For example, Google will not let us install from pip in CI. We have CIPD which manages our packages. So I was trying to think of a flexible way to default to the common package manager while allowing downstream to override.

@yperess
Copy link
Collaborator Author

yperess commented Feb 23, 2023

Adding to the thread some people that may be interested @fabiobaltieri @andyross @mbolivar

@yperess yperess changed the title West update dependency management West update tooling dependency management Feb 24, 2023
@fabiobaltieri
Copy link
Member

@yperess what would improve upstream from this? I think we discussed how this could help maintaining what right now is under requirements*.txt, maybe help out tracing what needs a certain dependency (which may be use to find what is no longer needed) and simplifying the CI setup.

@yperess
Copy link
Collaborator Author

yperess commented Feb 28, 2023

@yperess what would improve upstream from this? I think we discussed how this could help maintaining what right now is under requirements*.txt, maybe help out tracing what needs a certain dependency (which may be use to find what is no longer needed) and simplifying the CI setup.

Upstream would benefit from:

  1. Managing dependencies in one way. Currently we use requirements.txt for Python and plain documentation for native. This means that adding a new dependency causes developers to fumble around and find what's missing after a west update.
  2. If a dependency is for one of Zephyr's modules, it will live there. That means that if another project doesn't require the module, it won't pull the dependencies for it.
  3. It could automate creating, using, and updating the Python virtual environment.
  4. It's flexible so if you want to use a different mirror or host for the dependencies (a requirement for some bigger companies) you can.

All of this will make things easier to set up CI and generally develop for Zephyr over a long period of time.

@cfriedt
Copy link
Member

cfriedt commented Feb 28, 2023

Touched upon here as well
#54276

Some copy-paste

  • augment module yaml schema with necessary packages
    • ubuntu: (array of deb package-names..)
    • macos: (array of package-names..)
    • windows: (array of package-names..)
    • pip: (array of package-names..)
    • redhat: (array of package-names..)
  • each of the above would of course be optional, but every module would be required to list its own dependencies outside of the most basic requirements for building Zephyr.

@galak
Copy link
Collaborator

galak commented Feb 28, 2023

is something like https://github.com/kdeldycke/meta-package-manager be useful to abstract the different pkg/os managers?

@mbolivar-nordic
Copy link
Contributor

mbolivar-nordic commented Feb 28, 2023

During Arch WG, @yperess said that it doesn't have to go into west update, it could go into its own command, etc.

I want to encourage not putting it into west update for the following reasons off the top of my head:

  • every time we have coupled something zephyr-specific into 'core' west, it has turned out to be a mistake; I suspect this is no different. west update has no knowledge of module.yml and I think the coupling should be kept loose unless there is a convincing reason why not
  • subclassing built-in commands and silently replacing the implementation is a hard no from me. This has the same security problems as Allow to execute post update script for every project west#320, which nobody has successfully convinced me can be avoided. Users need to be able to trust that west update does exactly what the documentation says and what they see in the code for the pypi package they installed
  • people are going to want to hack this up for their own internal environments, so as a corollary to the previous point, putting this in an extension makes it easier for them to do so without having to fork west

@mbolivar-nordic
Copy link
Contributor

mbolivar-nordic commented Feb 28, 2023

@yperess as a general design request, I'd like to see more information on the layering where the changes to 'core' west are all mechanism, and the uses in zephyr are the application of those mechanisms towards some particular policy that achieves the desired goal here.

@yperess
Copy link
Collaborator Author

yperess commented Mar 13, 2023

@mbolivar-nordic thanks for the feedback and your patience. I get your point about west update and honestly I only thought of adding it there because it would be nice not to have to remember another command to run EVERY time after I run update. For now I'm happy adding this as a standalone command. So I guess I'll amend my proposal to:

  1. Add west workspace --list-requirements for requirements of the "current" workspace. This subcommand will list all the requirements as curated by the modules. @galak I like mpm, I'll try to use their json format and see what happens.
  2. Add west workspace which should:
    2.1 Get the list of requirements
    2.2 Check if they're all up-to-date (update if not)
    2.3 Create or enter the python virtual environment (default would be .zenv but can be overridden in the west config).

I'm currently only really fuzzy on the last step of using python to enter the virtual environment. It might be impossible but someone with more Python experience should probably weigh in.

@mbolivar-nordic
Copy link
Contributor

Using the name west workspace for an extension command feels like stealing prime namespace for a builtin. Can we try to come up with something better? How about west modules instead?

It might be impossible

It is impossible to enter a virtual environment from a west command.

Entering a virtual environment involves changing environment variables. Child processes cannot change the environments of their parents. West therefore cannot change the shell environment it runs under.

This is why the venv module creates multiple activation scripts, one per supported calling environment (bash, .bat, etc.), and it's why you have to source, not run, the scripts.

@mbolivar-nordic
Copy link
Contributor

@MaureenHelm MaureenHelm assigned mbolivar-nordic 5 days ago

Not me. @yperess feel free to reopen and assign yourself

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: West West utility RFC Request For Comments: want input from the community
Projects
Status: Done
Development

No branches or pull requests

8 participants