Bug 42036 - svn-unapply warns of uninitialized variable when unapplying a patch that describes an svn move operation
Summary: svn-unapply warns of uninitialized variable when unapplying a patch that desc...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Daniel Bates
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-07-10 15:52 PDT by Daniel Bates
Modified: 2010-07-26 13:58 PDT (History)
4 users (show)

See Also:


Attachments
Example (984 bytes, patch)
2010-07-10 15:53 PDT, Daniel Bates
no flags Details | Formatted Diff | Diff
Patch (2.40 KB, patch)
2010-07-10 16:20 PDT, Daniel Bates
no flags Details | Formatted Diff | Diff
Alternative Patch (1.98 KB, patch)
2010-07-24 12:37 PDT, Daniel Bates
no flags Details | Formatted Diff | Diff
Alternative Patch (2.02 KB, patch)
2010-07-24 12:45 PDT, Daniel Bates
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Daniel Bates 2010-07-10 15:52:24 PDT
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.
Comment 1 Daniel Bates 2010-07-10 15:53:35 PDT
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.
Comment 2 Daniel Bates 2010-07-10 16:20:42 PDT
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.
Comment 3 Daniel Bates 2010-07-24 12:37:57 PDT
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.
Comment 4 Daniel Bates 2010-07-24 12:45:36 PDT
Created attachment 62509 [details]
Alternative Patch

Updated change log.
Comment 5 Daniel Bates 2010-07-26 13:58:17 PDT
Comment on attachment 62509 [details]
Alternative Patch

Clearing flags on attachment: 62509

Committed r64072: <http://trac.webkit.org/changeset/64072>
Comment 6 Daniel Bates 2010-07-26 13:58:26 PDT
All reviewed patches have been landed.  Closing bug.