Summary

Fix up SVN patcher behavior when basedirs don't overlap.

Review Request #14343 — Created Feb. 12, 2025 and submitted March 14, 2025, 6:11 p.m.

Information

Owner

david

Repository

RBTools

Branch

release-5.x

Bugs

Depends On

Reviewers

Groups

rbtools

People

Description

Unfortunately, this completely broke the use case where a patch is
created in a directory that has no overlap at all with the client (for
example, the patch was created in /trunk/project1/ but the local
checkout is /branches/release/project1/). In this case, we'd filter out
everything, attempt to apply an empty patch, and error out.

This change updates the patcher behavior to be smarter about how base
directories are dealt with. If the client is a subdirectory of the patch
base directory, we do exactly as before: filter out any files from the
patch which are not present in the client's root, and try to patch. In
all other cases, we now do two new things:

The prefix level will be computed from the patch base directory
instead of the client's.
We just take the patch as-is (with the computed prefix level) and try
to apply it. If there are any conflicts, it'll be up to the user to
figure out why.

Testing Done


Ran unit tests, including two new tests for the subdirectory and

  no-overlap scenarios.
Checked out /trunk/ and /branches/1.14.x/ from the apache SVN

  repository. Posted one change from trunk, and then used rbt patch to

  apply it into the branch. Before this would error out with the empty

  patch, after it works correctly.

Commits

Summary	ID
Fix up SVN patcher behavior when basedirs don't overlap. The new SVN patcher implementation tried to be very smart when the patch base directory was not the same as the client base directory, filtering out files that are not patchable. This would happen, for example, if a patch was created in /trunk/ but the local checkout only contained /trunk/project1/. Unfortunately, this completely broke the use case where a patch is created in a directory that has no overlap at all with the client (for example, the patch was created in /trunk/project1/ but the local checkout is /branches/release/project1/). In this case, we'd filter out everything, attempt to apply an empty patch, and error out. This change updates the patcher behavior to be smarter about how base directories are dealt with. If the client is a subdirectory of the patch base directory, we do exactly as before: filter out any files from the patch which are not present in the client's root, and try to patch. In all other cases, we now do two new things: 1. The prefix level will be computed from the patch base directory instead of the client's. 2. We just take the patch as-is (with the computed prefix level) and try to apply it. If there are any conflicts, it'll be up to the user to figure out why. Testing Done: - Ran unit tests, including two new tests for the subdirectory and no-overlap scenarios. - Checked out /trunk/ and /branches/1.14.x/ from the apache SVN repository. Posted one change from trunk, and then used `rbt patch` to apply it into the branch. Before this would error out with the empty patch, after it works correctly.	bd989b6d99044fab476df564f0c8a07571b2b76c

Summary

Fix up SVN patcher behavior when basedirs don't overlap.

The new SVN patcher implementation tried to be very smart when the patch base directory was not the same as the client base directory, filtering out files that are not patchable. This would happen, for example, if a patch was created in /trunk/ but the local checkout only contained /trunk/project1/. Unfortunately, this completely broke the use case where a patch is created in a directory that has no overlap at all with the client (for example, the patch was created in /trunk/project1/ but the local checkout is /branches/release/project1/). In this case, we'd filter out everything, attempt to apply an empty patch, and error out. This change updates the patcher behavior to be smarter about how base directories are dealt with. If the client is a subdirectory of the patch base directory, we do exactly as before: filter out any files from the patch which are not present in the client's root, and try to patch. In all other cases, we now do two new things: 1. The prefix level will be computed from the patch base directory instead of the client's. 2. We just take the patch as-is (with the computed prefix level) and try to apply it. If there are any conflicts, it'll be up to the user to figure out why. Testing Done: - Ran unit tests, including two new tests for the subdirectory and no-overlap scenarios. - Checked out /trunk/ and /branches/1.14.x/ from the apache SVN repository. Posted one change from trunk, and then used `rbt patch` to apply it into the branch. Before this would error out with the empty patch, after it works correctly.

bd989b6d99044fab476df564f0c8a07571b2b76c

Issues

Description	From	Last Updated
line too long (80 > 79 characters) Column: 80 Error code: E501	reviewbot	Feb. 12, 2025, 11:32 a.m.
Add , optional	maubin	Feb. 13, 2025, 11:25 a.m.
The second line of the bullet point must be aligned with the content of the first line.	chipx86	March 14, 2025, 3:47 p.m.
Missing Version Added.	chipx86	March 14, 2025, 3:47 p.m.
Rather than invoking the whole command twice, let's just compute what base_dir value we want to use.	chipx86	March 14, 2025, 3:47 p.m.
Should probably be a Sequence.	chipx86	March 14, 2025, 5 p.m.
undefined name 'Sequence' Column: 18 Error code: F821	reviewbot	March 14, 2025, 5:33 p.m.

flake8 failed.

JSHint passed.

flake8

rbtools/clients/svn.py (Diff revision 1)
The issue has been dropped. Show all issues
```
line too long (80 > 79 characters)

Column: 80
Error code: E501
```

Commits:

	Summary	ID
	Fix up SVN patcher behavior when basedirs don't overlap. The new SVN patcher implementation tried to be very smart when the patch base directory was not the same as the client base directory, filtering out files that are not patchable. This would happen, for example, if a patch was created in /trunk/ but the local checkout only contained /trunk/project1/. Unfortunately, this completely broke the use case where a patch is created in a directory that has no overlap at all with the client (for example, the patch was created in /trunk/project1/ but the local checkout is /branches/release/project1/). In this case, we'd filter out everything, attempt to apply an empty patch, and error out. This change updates the patcher behavior to be smarter about how base directories are dealt with. If the client is a subdirectory of the patch base directory, we do exactly as before: filter out any files from the patch which are not present in the client's root, and try to patch. In all other cases, we now do two new things: 1. The prefix level will be computed from the patch base directory instead of the client's. 2. We just take the patch as-is (with the computed prefix level) and try to apply it. If there are any conflicts, it'll be up to the user to figure out why. Testing Done: - Ran unit tests, including two new tests for the subdirectory and no-overlap scenarios. - Checked out /trunk/ and /branches/1.14.x/ from the apache SVN repository. Posted one change from trunk, and then used `rbt patch` to apply it into the branch. Before this would error out with the empty patch, after it works correctly.	52a401727156494414d8afc091726aabcce9a97c
	Fix up SVN patcher behavior when basedirs don't overlap. The new SVN patcher implementation tried to be very smart when the patch base directory was not the same as the client base directory, filtering out files that are not patchable. This would happen, for example, if a patch was created in /trunk/ but the local checkout only contained /trunk/project1/. Unfortunately, this completely broke the use case where a patch is created in a directory that has no overlap at all with the client (for example, the patch was created in /trunk/project1/ but the local checkout is /branches/release/project1/). In this case, we'd filter out everything, attempt to apply an empty patch, and error out. This change updates the patcher behavior to be smarter about how base directories are dealt with. If the client is a subdirectory of the patch base directory, we do exactly as before: filter out any files from the patch which are not present in the client's root, and try to patch. In all other cases, we now do two new things: 1. The prefix level will be computed from the patch base directory instead of the client's. 2. We just take the patch as-is (with the computed prefix level) and try to apply it. If there are any conflicts, it'll be up to the user to figure out why. Testing Done: - Ran unit tests, including two new tests for the subdirectory and no-overlap scenarios. - Checked out /trunk/ and /branches/1.14.x/ from the apache SVN repository. Posted one change from trunk, and then used `rbt patch` to apply it into the branch. Before this would error out with the empty patch, after it works correctly.	dbe48f15d9f4db0e974d936cfef0ace4dafad741

Diff:

Revision 2 (+458 -106)

Show changes

	rbtools/clients/svn.py
	rbtools/clients/tests/test_svn.py

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

Ship it!

rbtools/clients/svn.py (Diff revision 2)
The issue has been resolved. Show all issues
```
Add , optional
```

Commits:

	Summary	ID
	Fix up SVN patcher behavior when basedirs don't overlap. The new SVN patcher implementation tried to be very smart when the patch base directory was not the same as the client base directory, filtering out files that are not patchable. This would happen, for example, if a patch was created in /trunk/ but the local checkout only contained /trunk/project1/. Unfortunately, this completely broke the use case where a patch is created in a directory that has no overlap at all with the client (for example, the patch was created in /trunk/project1/ but the local checkout is /branches/release/project1/). In this case, we'd filter out everything, attempt to apply an empty patch, and error out. This change updates the patcher behavior to be smarter about how base directories are dealt with. If the client is a subdirectory of the patch base directory, we do exactly as before: filter out any files from the patch which are not present in the client's root, and try to patch. In all other cases, we now do two new things: 1. The prefix level will be computed from the patch base directory instead of the client's. 2. We just take the patch as-is (with the computed prefix level) and try to apply it. If there are any conflicts, it'll be up to the user to figure out why. Testing Done: - Ran unit tests, including two new tests for the subdirectory and no-overlap scenarios. - Checked out /trunk/ and /branches/1.14.x/ from the apache SVN repository. Posted one change from trunk, and then used `rbt patch` to apply it into the branch. Before this would error out with the empty patch, after it works correctly.	dbe48f15d9f4db0e974d936cfef0ace4dafad741
	Fix up SVN patcher behavior when basedirs don't overlap. The new SVN patcher implementation tried to be very smart when the patch base directory was not the same as the client base directory, filtering out files that are not patchable. This would happen, for example, if a patch was created in /trunk/ but the local checkout only contained /trunk/project1/. Unfortunately, this completely broke the use case where a patch is created in a directory that has no overlap at all with the client (for example, the patch was created in /trunk/project1/ but the local checkout is /branches/release/project1/). In this case, we'd filter out everything, attempt to apply an empty patch, and error out. This change updates the patcher behavior to be smarter about how base directories are dealt with. If the client is a subdirectory of the patch base directory, we do exactly as before: filter out any files from the patch which are not present in the client's root, and try to patch. In all other cases, we now do two new things: 1. The prefix level will be computed from the patch base directory instead of the client's. 2. We just take the patch as-is (with the computed prefix level) and try to apply it. If there are any conflicts, it'll be up to the user to figure out why. Testing Done: - Ran unit tests, including two new tests for the subdirectory and no-overlap scenarios. - Checked out /trunk/ and /branches/1.14.x/ from the apache SVN repository. Posted one change from trunk, and then used `rbt patch` to apply it into the branch. Before this would error out with the empty patch, after it works correctly.	6fb5c396bda71236511e4ba5a53130c27836cfaa

Diff:

Revision 3 (+458 -106)

Show changes

	rbtools/clients/svn.py
	rbtools/clients/tests/test_svn.py

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

Ship it!

```
Ship It!
```

rbtools/clients/svn.py (Diff revision 3)

The issue has been resolved. Show all issues

The second line of the bullet point must be aligned with the content of the first line.

rbtools/clients/svn.py (Diff revision 3)
The issue has been resolved. Show all issues
```
Missing Version Added.
```

rbtools/clients/svn.py (Diff revision 3)

The issue has been resolved. Show all issues

Rather than invoking the whole command twice, let's just compute what base_dir value we want to use.

rbtools/clients/tests/test_svn.py (Diff revision 3)
The issue has been resolved. Show all issues
```
Should probably be a Sequence.
```

Commits:

	Summary	ID
	Fix up SVN patcher behavior when basedirs don't overlap. The new SVN patcher implementation tried to be very smart when the patch base directory was not the same as the client base directory, filtering out files that are not patchable. This would happen, for example, if a patch was created in /trunk/ but the local checkout only contained /trunk/project1/. Unfortunately, this completely broke the use case where a patch is created in a directory that has no overlap at all with the client (for example, the patch was created in /trunk/project1/ but the local checkout is /branches/release/project1/). In this case, we'd filter out everything, attempt to apply an empty patch, and error out. This change updates the patcher behavior to be smarter about how base directories are dealt with. If the client is a subdirectory of the patch base directory, we do exactly as before: filter out any files from the patch which are not present in the client's root, and try to patch. In all other cases, we now do two new things: 1. The prefix level will be computed from the patch base directory instead of the client's. 2. We just take the patch as-is (with the computed prefix level) and try to apply it. If there are any conflicts, it'll be up to the user to figure out why. Testing Done: - Ran unit tests, including two new tests for the subdirectory and no-overlap scenarios. - Checked out /trunk/ and /branches/1.14.x/ from the apache SVN repository. Posted one change from trunk, and then used `rbt patch` to apply it into the branch. Before this would error out with the empty patch, after it works correctly.	6fb5c396bda71236511e4ba5a53130c27836cfaa
	Fix up SVN patcher behavior when basedirs don't overlap. The new SVN patcher implementation tried to be very smart when the patch base directory was not the same as the client base directory, filtering out files that are not patchable. This would happen, for example, if a patch was created in /trunk/ but the local checkout only contained /trunk/project1/. Unfortunately, this completely broke the use case where a patch is created in a directory that has no overlap at all with the client (for example, the patch was created in /trunk/project1/ but the local checkout is /branches/release/project1/). In this case, we'd filter out everything, attempt to apply an empty patch, and error out. This change updates the patcher behavior to be smarter about how base directories are dealt with. If the client is a subdirectory of the patch base directory, we do exactly as before: filter out any files from the patch which are not present in the client's root, and try to patch. In all other cases, we now do two new things: 1. The prefix level will be computed from the patch base directory instead of the client's. 2. We just take the patch as-is (with the computed prefix level) and try to apply it. If there are any conflicts, it'll be up to the user to figure out why. Testing Done: - Ran unit tests, including two new tests for the subdirectory and no-overlap scenarios. - Checked out /trunk/ and /branches/1.14.x/ from the apache SVN repository. Posted one change from trunk, and then used `rbt patch` to apply it into the branch. Before this would error out with the empty patch, after it works correctly.	bd989b6d99044fab476df564f0c8a07571b2b76c

Diff:

Revision 4 (+470 -110)

Show changes

	rbtools/clients/svn.py
	rbtools/clients/tests/test_svn.py

Checks run (1 failed, 1 succeeded)

flake8 failed.

JSHint passed.

flake8

rbtools/clients/tests/test_svn.py (Diff revision 4)
The issue has been resolved. Show all issues
```
undefined name 'Sequence'

Column: 18
Error code: F821
```

Ship it!

```
I'm happy once Review Bot is happy.
```

Status:: Completed
Change Summary:: Pushed to release-5.x (e7e6b73)