Add the beginnings of support for dependencies for SCMClients.

Review Request #12529 — Created Aug. 13, 2022 and submitted

Information

RBTools
release-4.x

Reviewers

All of our SCMClients need to check for compatible tools to wrap, but
we've never really had a formal way of doing these checks. We eventually
sort of standardized on doing them in get_local_path(), but this both
meant that tools couldn't be checked up-front, and that tools would
re-check every time we wanted to check the path. We dealt with this
through caching of information, but it wasn't great.

This approach also had a problem where get_local_path() was doing the
GNU diff checks. With Apple Diff support coming, we needed a better way
of checking the diff tool, outside of the SCMClient. That led to a
rethink on dependency checks.

SCMClients should now implement check_dependencies() to do their tool
existence/version checks. This method must raise a
ClientDependencyError exception if all mandatory dependencies fail.

It may store any data on the instance or on a class to indicate, for
example, which executable name or tool version it's working with.

It also must not check for GNU diff, as that will be a check performed
outside of the SCMClients.

When instantiating a tool, it's now required to run either
scm_client.setup() or scm_client.has_dependencies().

BaseSCMClient.setup() will check for dependencies the first time it's
called and will store whether the tool has dependencies. If dependency
checks fail, it will re-raise the exception. Callers should explicitly
call setup() if they are interested in the details of the missing
dependencies.

This method must not be overridden. If SCMClients need additional setup
hooks, that's something we'd need to provide separately for setup() to
call.

BaseSCMClient.has_dependencies() will call setup() if needed. It
then returns a boolean indicating if dependencies are present. This is a
good alternative if the caller just needs to check the usability of a
tool before doing anything else with it.

As of RBTools 4.0, clients can be used without calling setup(), but a
deprecation warning will be issued. This will help transition to the new
behavior. This takes place in has_dependencies() with the default
parameter of expect_checked=True. In RBTools 5, this will raise an
exception instead.

Upcoming changes will begin to transition over to this for specifying
dependencies, for simplifying get_local_path(), and for calling
setup().

Unit tests pass on Python 3.7-3.11.

Summary ID
Add the beginnings of support for dependencies for SCMClients.
All of our SCMClients need to check for compatible tools to wrap, but we've never really had a formal way of doing these checks. We eventually sort of standardized on doing them in `get_local_path()`, but this both meant that tools couldn't be checked up-front, and that tools would re-check every time we wanted to check the path. We dealt with this through caching of information, but it wasn't great. This approach also had a problem where `get_local_path()` was doing the GNU diff checks. With Apple Diff support coming, we needed a better way of checking the diff tool, outside of the SCMClient. That led to a rethink on dependency checks. SCMClients should now implement `check_dependencies()` to do their tool existence/version checks. This method must raise a `ClientDependencyError` exception if all mandatory dependencies fail. It may store any data on the instance or on a class to indicate, for example, which executable name or tool version it's working with. It also must *not* check for GNU diff, as that will be a check performed outside of the SCMClients. When instantiating a tool, it's now required to run either `scm_client.setup()` or `scm_client.has_dependencies()`. `BaseSCMClient.setup()` will check for dependencies the first time it's called and will store whether the tool has dependencies. If dependency checks fail, it will re-raise the exception. Callers should explicitly call `setup()` if they are interested in the details of the missing dependencies. This method must not be overridden. If SCMClients need additional setup hooks, that's something we'd need to provide separately for `setup()` to call. `BaseSCMClient.has_dependencies()` will call `setup()` if needed. It then returns a boolean indicating if dependencies are present. This is a good alternative if the caller just needs to check the usability of a tool before doing anything else with it. As of RBTools 4.0, clients can be used without calling `setup()`, but a deprecation warning will be issued. This will help transition to the new behavior. This takes place in `has_dependencies()` with the default parameter of `expect_checked=True`. In RBTools 5, this will raise an exception instead. Upcoming changes will begin to transition over to this for specifying dependencies, for simplifying `get_local_path()`, and for calling `setup()`.
e2350d5474d73233138777cd43979b3261fae381
Description From Last Updated

'typing.Any' imported but unused Column: 1 Error code: F401

reviewbotreviewbot
Checks run (1 failed, 1 succeeded)
flake8 failed.
JSHint passed.

flake8

chipx86
david
  1. Ship It!
  2. 
      
chipx86
Review request changed

Status: Closed (submitted)

Change Summary:

Pushed to release-4.x (923d340)
Loading...