Bug 42036 – svn-unapply warns of uninitialized variable when unapplying a patch that describes an svn move operation

• Component: Tools / Tests
• Status: RESOLVED FIXED

[Daniel Bates]

When using svn-unapply to unapply a patch that describes an svn move operation, svn-unapply gave me the following errors:

Use of uninitialized value $patch in pattern match (m//) at WebKitTools/Scripts/svn-unapply line 153.
Use of uninitialized value $patch in print at /Users/dbates/Desktop/WebKit/WebKitTools/Scripts/VCSUtils.pm line 1433.

[Daniel Bates]

Created attachment 61166 [details]
Example

This is an example patch that demonstrates the issue.

Apply this patch using svn-apply. Then un-apply it using svn-unapply.

[Daniel Bates]

Created attachment 61168 [details]
Patch

The issue is that for an SVN copy/move  we do not pass the hash key svnConvertedText in the hash we pass to the patch() function because it is not necessary when performing such an operation (i.e. there are no additional changes to unapply to the copied/moved file; we are just going to remove the file. If there were additional changes made to the copied/moved file, as described by the patch we are unapplying, these would appear as separate diffs that would have already been unapplied).

I'm unclear how best to test since it it very specific to svn-unapply and patch() makes file system calls which we would need to either mock out or actually perform similar to the way we do the WebKit python SCM tests. I am open to suggestions.

[Daniel Bates]

Created attachment 62508 [details]
Alternative Patch

I noticed that svn-apply initializes $patch to "" for a similar issue <http://trac.webkit.org/browser/trunk/WebKitTools/Scripts/svn-apply?rev=59344#L300>, so I came up with this alternative patch.

I am open to suggestions if we should use the first patch <https://bugs.webkit.org/attachment.cgi?id=61168&action=edit> or this patch or some other patch to resolve this bug. The first patch has a bit more specificity in that it checks for the existence of the key copiedFromPath, which always exists if the diff represents a svn copy/move operation.

[Daniel Bates]

Created attachment 62509 [details]
Alternative Patch

Updated change log.

[Daniel Bates]

Comment on attachment 62509 [details]
Alternative Patch

Clearing flags on attachment: 62509

Committed r64072: <http://trac.webkit.org/changeset/64072>

[Daniel Bates]

All reviewed patches have been landed.  Closing bug.