Switch RBTools to use modern patching, and add a legacy patch wrapper.

Review Request #14224 — Created Nov. 3, 2024 and submitted

Information

RBTools
release-5.x

Reviewers

rbt patch and rbt land now use the new Patcher class
(specifically, a SCMClientPatcher), which gives SCMClients a lot more
control over the patching and committing process.

By default, SCMClients that don't define custom patching logic will use
the built-in patching logic that leverages the patch tool.

SCMClients that override apply_patch() will continue to use their
existing patching logic, but wrapped by a _LegacyPatcher
implementation that'll maintain compatibility until RBTools 7. This acts
as a bridge between the new Patcher API and legacy implementations of
SCMClient.apply_patch() and SCMClient.apply_patch_for_empty_files()

rbt patch and rbt land will call SCMClient.get_patcher() to return
a SCMClientPatcher-compatible instance for handling patching
operations.

Now, there's a little bit of a complex transition here.
SCMClient.apply_patch() will remain a valid function for applying
patches for the time-being, but its behavior changes between the legacy
patcher and the modern patcher:

  • By default, a SCMClient that doesn't implement custom patching
    behavior) will use a default apply_patch() method that gets a
    default SCMClientPatcher class for handling patches by way of
    get_patcher().

  • Updated SCMClients with custom patching behavior are expected to now
    subclass Patcher and override patcher_cls to match it.
    get_patcher() will return it, and apply_patch() will interface
    with it.

  • SCMClients that override apply_patch() with legacy patching behavior
    will behave as they did before, for now, and get_patcher() will
    return a _LegacyPatcher to wrap it. This enables rbt patch to work
    with the legacy implementation.

Over time, we'll remove _LegacyPatcher. We may also be deprecating
apply_patch() soon, once everything is transitioned over.

All unit tests passed.

Applied patches to local trees using both the default patching
implementation and the legacy patcher. Tested applying, reversing,
and committing.

Summary ID
Switch RBTools to use modern patching, and add a legacy patch wrapper.
`rbt patch` and `rbt land` now use the new `Patcher` class (specifically, a `SCMClientPatcher`), which gives SCMClients a lot more control over the patching and committing process. By default, SCMClients that don't define custom patching logic will use the built-in patching logic that leverages the `patch` tool. SCMClients that override `apply_patch()` will continue to use their existing patching logic, but wrapped by a `_LegacyPatcher` implementation that'll maintain compatibility until RBTools 7. This acts as a bridge between the new `Patcher` API and legacy implementations of `SCMClient.apply_patch()` and `SCMClient.apply_patch_for_empty_files()` `rbt patch` and `rbt land` will call `SCMClient.get_patcher()` to return a `SCMClientPatcher`-compatible instance for handling patching operations. Now, there's a little bit of a complex transition here. `SCMClient.apply_patch()` will remain a valid function for applying patches for the time-being, but its behavior changes between the legacy patcher and the modern patcher: * By default, a SCMClient that doesn't implement custom patching behavior) will use a default `apply_patch()` method that gets a default `SCMClientPatcher` class for handling patches by way of `get_patcher()`. * Updated SCMClients with custom patching behavior are expected to now subclass `Patcher` and override `patcher_cls` to match it. `get_patcher()` will return it, and `apply_patch()` will interface with it. * SCMClients that override `apply_patch()` with legacy patching behavior will behave as they did before, for now, and `get_patcher()` will return a `_LegacyPatcher` to wrap it. This enables `rbt patch` to work with the legacy implementation. Over time, we'll remove `_LegacyPatcher`. We may also be deprecating `apply_patch()` soon, once everything is transitioned over.
2eb68f653d4e0d0591c5b6966e890517a6cedf64
Description From Last Updated

'rbtools.clients.base.scmclient.BaseSCMClient' imported but unused Column: 1 Error code: F401

reviewbotreviewbot

'rbtools.diffs.patcher.Patcher' imported but unused Column: 1 Error code: F401

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

flake8

chipx86
chipx86
Review request changed
Change Summary:

Added a SCMClientPatcher base class for SCMClient-owned patchers.

Description:
~  

rbt patch and rbt land now use the new Patcher class, which gives

~   SCMClients a lot more control over the patching and committing process.

  ~

rbt patch and rbt land now use the new Patcher class

  ~ (specifically, a SCMClientPatcher), which gives SCMClients a lot more
  + control over the patching and committing process.

   
~  

By default, SCMClients will use their existing patching logic, wrapped

~   by a _LegacyPatcher implementation that'll be used by default until
~   RBTools 7. This acts as a bridge between the new Patcher API and
~   legacy implementations of SCMClient.apply_patch() and
~   SCMClient.apply_patch_for_empty_files()

  ~

By default, SCMClients that don't define custom patching logic will use

  ~ the built-in patching logic that leverages the patch tool.

  ~
  ~

SCMClients that override apply_patch() will continue to use their

  ~ existing patching logic, but wrapped by a _LegacyPatcher
  + implementation that'll maintain compatibility until RBTools 7. This acts
  + as a bridge between the new Patcher API and legacy implementations of
  + SCMClient.apply_patch() and SCMClient.apply_patch_for_empty_files()

   
   

rbt patch and rbt land will call SCMClient.get_patcher() to return

~   a Patcher-compatible instance for handling patching operations.

  ~ a SCMClientPatcher-compatible instance for handling patching
  + operations.

   
   

Now, there's a little bit of a complex transition here.

    SCMClient.apply_patch() will remain a valid function for applying
    patches for the time-being, but its behavior changes between the legacy
    patcher and the modern patcher:

   
~  
  • By default, a SCMClient that doesn't implement custom patching
    behavior) will use a default apply_patch() method that gets a
    default Patcher class for handling patches by way of
    get_patcher().

  ~
  • By default, a SCMClient that doesn't implement custom patching
    behavior) will use a default apply_patch() method that gets a
    default SCMClientPatcher class for handling patches by way of
    get_patcher().

   
  • Updated SCMClients with custom patching behavior are expected to now
    subclass Patcher and override patcher_cls to match it.
    get_patcher() will return it, and apply_patch() will interface
    with it.

   
  • SCMClients that override apply_patch() with legacy patching behavior
    will behave as they did before, for now, and get_patcher() will
    return a _LegacyPatcher to wrap it. This enables rbt patch to work
    with the legacy implementation.

   
   

Over time, we'll remove _LegacyPatcher. We may also be deprecating

    apply_patch() soon, once everything is transitioned over.

Commits:
Summary ID
Switch RBTools to use modern patching, and add a legacy patch wrapper.
`rbt patch` and `rbt land` now use the new `Patcher` class, which gives SCMClients a lot more control over the patching and committing process. By default, SCMClients will use their existing patching logic, wrapped by a `_LegacyPatcher` implementation that'll be used by default until RBTools 7. This acts as a bridge between the new `Patcher` API and legacy implementations of `SCMClient.apply_patch()` and `SCMClient.apply_patch_for_empty_files()` `rbt patch` and `rbt land` will call `SCMClient.get_patcher()` to return a `Patcher`-compatible instance for handling patching operations. Now, there's a little bit of a complex transition here. `SCMClient.apply_patch()` will remain a valid function for applying patches for the time-being, but its behavior changes between the legacy patcher and the modern patcher: * By default, a SCMClient that doesn't implement custom patching behavior) will use a default `apply_patch()` method that gets a default `Patcher` class for handling patches by way of `get_patcher()`. * Updated SCMClients with custom patching behavior are expected to now subclass `Patcher` and override `patcher_cls` to match it. `get_patcher()` will return it, and `apply_patch()` will interface with it. * SCMClients that override `apply_patch()` with legacy patching behavior will behave as they did before, for now, and `get_patcher()` will return a `_LegacyPatcher` to wrap it. This enables `rbt patch` to work with the legacy implementation. Over time, we'll remove `_LegacyPatcher`. We may also be deprecating `apply_patch()` soon, once everything is transitioned over.
a6694baa951170a39e92f2e614ba5285c0149bfe
Switch RBTools to use modern patching, and add a legacy patch wrapper.
`rbt patch` and `rbt land` now use the new `Patcher` class (specifically, a `SCMClientPatcher`), which gives SCMClients a lot more control over the patching and committing process. By default, SCMClients that don't define custom patching logic will use the built-in patching logic that leverages the `patch` tool. SCMClients that override `apply_patch()` will continue to use their existing patching logic, but wrapped by a `_LegacyPatcher` implementation that'll maintain compatibility until RBTools 7. This acts as a bridge between the new `Patcher` API and legacy implementations of `SCMClient.apply_patch()` and `SCMClient.apply_patch_for_empty_files()` `rbt patch` and `rbt land` will call `SCMClient.get_patcher()` to return a `SCMClientPatcher`-compatible instance for handling patching operations. Now, there's a little bit of a complex transition here. `SCMClient.apply_patch()` will remain a valid function for applying patches for the time-being, but its behavior changes between the legacy patcher and the modern patcher: * By default, a SCMClient that doesn't implement custom patching behavior) will use a default `apply_patch()` method that gets a default `SCMClientPatcher` class for handling patches by way of `get_patcher()`. * Updated SCMClients with custom patching behavior are expected to now subclass `Patcher` and override `patcher_cls` to match it. `get_patcher()` will return it, and `apply_patch()` will interface with it. * SCMClients that override `apply_patch()` with legacy patching behavior will behave as they did before, for now, and `get_patcher()` will return a `_LegacyPatcher` to wrap it. This enables `rbt patch` to work with the legacy implementation. Over time, we'll remove `_LegacyPatcher`. We may also be deprecating `apply_patch()` soon, once everything is transitioned over.
fbfdc1b33f56660d654144653724ec0e85d68c83

Checks run (1 failed, 1 succeeded)

flake8 failed.
JSHint passed.

flake8

chipx86
chipx86
david
  1. Ship It!
  2. 
      
maubin
  1. Ship It!
  2. 
      
chipx86
Review request changed
Status:
Completed
Change Summary:
Pushed to release-5.x (8bc6d38)