Bug 167264 - svn-create-patch should emit properties when files are moved or copied
Summary: svn-create-patch should emit properties when files are moved or copied
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Jonathan Bedard
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2017-01-20 15:56 PST by Jonathan Bedard
Modified: 2017-01-23 12:16 PST (History)
4 users (show)

See Also:

Attachments
Patch (1.30 KB, patch)
2017-01-20 16:07 PST, Jonathan Bedard 		no flags Details | Formatted Diff | Diff
Patch (1.22 KB, patch)
2017-01-20 17:20 PST, Jonathan Bedard 		no flags Details | Formatted Diff | Diff
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Jonathan Bedard 2017-01-20 15:56:47 PST 
svn-create-patch does not emit properties when files are moved or copied through svn move or svn copy.
Comment 1 Jonathan Bedard 2017-01-20 16:07:35 PST 
Created attachment 299401 [details]
Patch
Comment 2 Jonathan Bedard 2017-01-20 17:20:46 PST 
Created attachment 299408 [details]
Patch
Comment 3 Jonathan Bedard 2017-01-20 17:23:43 PST 
(In reply to comment #2)
> Created attachment 299408 [details]
> Patch

Minor change on the original patch, same result.

Note that without https://bugs.webkit.org/show_bug.cgi?id=167169, this code will fail to emit the correct patch in the following case:

svn mv file1 file2
svn propset <property> file2

In this case (and similar cases) the patch will result in file2 having the same properties as file1
Comment 4 Daniel Bates 2017-01-20 17:58:32 PST 
Comment on attachment 299408 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=299408&action=review

> Tools/Scripts/svn-create-patch:331
> -        print `svn cat ${escapedSourceFile} | diff -u $devNull - | tail -n +3`;
> +        print `svn diff -r 0:${sourceRevision} ${escapedSourceFile} | tail -n +5`;

I suspect that some (hopefully ancient) version of SVN did not support diffing against revision 0 and this led us to come up with the svn cat | diff workaround when we added this function in the patch for bug #12023. Is there a version of SVN that does not support diffing from revision 0? If so, do we need to support such an SVN version (say, for Windows, GTK or EFL ports)?

From my own testing, SVN version 1.7.4 (r1295709) and 1.9.4 (r1740329) support diffing against revision 0.
Comment 5 Jonathan Bedard 2017-01-23 09:38:57 PST 
Comment on attachment 299408 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=299408&action=review

>> Tools/Scripts/svn-create-patch:331
>> +        print `svn diff -r 0:${sourceRevision} ${escapedSourceFile} | tail -n +5`;
> 
> I suspect that some (hopefully ancient) version of SVN did not support diffing against revision 0 and this led us to come up with the svn cat | diff workaround when we added this function in the patch for bug #12023. Is there a version of SVN that does not support diffing from revision 0? If so, do we need to support such an SVN version (say, for Windows, GTK or EFL ports)?
> 
> From my own testing, SVN version 1.7.4 (r1295709) and 1.9.4 (r1740329) support diffing against revision 0.

I couldn't find any documentation that would indicate this ever didn't work.

If this did fail, it would likely look something like this:
svn: E#: Syntax error in revision argument '0:r#'
Comment 6 Daniel Bates 2017-01-23 10:52:52 PST 
Comment on attachment 299408 [details]
Patch

The Windows, GTK and EFL bots didn't complain. Let's try this!
Comment 7 Jonathan Bedard 2017-01-23 12:14:06 PST 
Comment on attachment 299408 [details]
Patch

Just as an additional note, it seems that we don't use this technique anywhere else, and talking with Dave Kilzer, this was not a technique considered originally.
Comment 8 WebKit Commit Bot 2017-01-23 12:16:32 PST 
Comment on attachment 299408 [details]
Patch

Clearing flags on attachment: 299408

Committed r211048: <http://trac.webkit.org/changeset/211048>
Comment 9 WebKit Commit Bot 2017-01-23 12:16:37 PST 
All reviewed patches have been landed.  Closing bug.