• 
      

    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:
    Completed
    Change Summary:
    Pushed to release-4.x (923d340)