Summary

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

Review Request #14224 — Created Nov. 3, 2024 and updated Nov. 6, 2024, 3:27 p.m.

Information

Owner

chipx86

Repository

RBTools

Branch

release-5.x

Bugs

Depends On

Blocks

14225

Reviewers

Groups

rbtools

People

Description

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.

Testing Done

All unit tests passed.

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

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 (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

Summary

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

Issues

Description	From	Last Updated
'rbtools.clients.base.scmclient.BaseSCMClient' imported but unused Column: 1 Error code: F401	reviewbot	Nov. 3, 2024, 10:33 p.m.
'rbtools.diffs.patcher.Patcher' imported but unused Column: 1 Error code: F401	reviewbot	Nov. 3, 2024, 11:04 p.m.

flake8 failed.

JSHint passed.

flake8

rbtools/commands/patch.py (Diff revision 1)

The issue has been resolved. Show all issues

'rbtools.clients.base.scmclient.BaseSCMClient' imported but unused

Column: 1
Error code: F401

Change Summary:

Removed unused imports.

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.	b4e43244593e476e3c2849cf280643296bad0d98
	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

Diff:

Revision 2 (+1392 -352)

Show changes

	rbtools/clients/base/scmclient.py
	rbtools/clients/tests/test_base_scmclient.py
	rbtools/clients/tests/test_legacy_patcher.py
	rbtools/commands/patch.py
	rbtools/diffs/patcher.py
	rbtools/diffs/tests/test_patcher.py

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

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

Diff:

Revision 3 (+1438 -352)

Show changes

	rbtools/clients/base/scmclient.py
	rbtools/clients/tests/test_base_scmclient.py
	rbtools/clients/tests/test_legacy_patcher.py
	rbtools/commands/patch.py
	rbtools/diffs/patcher.py
	rbtools/diffs/tests/test_patcher.py

Checks run (1 failed, 1 succeeded)

flake8 failed.

JSHint passed.

flake8

rbtools/clients/tests/test_base_scmclient.py (Diff revision 3)
The issue has been resolved. Show all issues
```
'rbtools.diffs.patcher.Patcher' imported but unused

Column: 1
Error code: F401
```

Change Summary:


Fixed an inversed status message ("apply" vs. "revert")
Fixed a long-standing patching bug where diff_revision (used in messages) wouldn't be set from the loaded diff.

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 (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
	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.	795c6d4cf24d4ce942675bf3c0bf51ba04dd304f

Diff:

Revision 4 (+1440 -352)

Show changes

	rbtools/clients/base/scmclient.py
	rbtools/clients/tests/test_base_scmclient.py
	rbtools/clients/tests/test_legacy_patcher.py
	rbtools/commands/patch.py
	rbtools/diffs/patcher.py
	rbtools/diffs/tests/test_patcher.py

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

Change Summary:

Moved the create_commit implementation from _LegacyPatcher into SCMClientPatcher.

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 (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.	795c6d4cf24d4ce942675bf3c0bf51ba04dd304f
	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

Diff:

Revision 5 (+1442 -352)

Show changes

	rbtools/clients/base/scmclient.py
	rbtools/clients/tests/test_base_scmclient.py
	rbtools/clients/tests/test_legacy_patcher.py
	rbtools/commands/patch.py
	rbtools/diffs/patcher.py
	rbtools/diffs/tests/test_patcher.py

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

Ship it!

```
Ship It!
```

Ship it!

```
Ship It!
```