Bug 29684 - svn-apply fails when a patch has an empty file
Summary: svn-apply fails when a patch has an empty file
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: David Kilzer (:ddkilzer)
URL:
Keywords: InRadar
: 38801 (view as bug list)
Depends on:
Blocks: 186009
  Show dependency treegraph
 
Reported: 2009-09-23 08:12 PDT by Eric Carlson
Modified: 2018-05-27 09:52 PDT (History)
7 users (show)

See Also:


Attachments
svn add empty file (93 bytes, patch)
2018-05-26 09:16 PDT, David Kilzer (:ddkilzer)
no flags Details | Formatted Diff | Diff
svn delete empty file (124 bytes, patch)
2018-05-26 09:17 PDT, David Kilzer (:ddkilzer)
no flags Details | Formatted Diff | Diff
git add empty file (103 bytes, patch)
2018-05-26 09:18 PDT, David Kilzer (:ddkilzer)
no flags Details | Formatted Diff | Diff
git delete empty file (169 bytes, patch)
2018-05-26 09:18 PDT, David Kilzer (:ddkilzer)
no flags Details | Formatted Diff | Diff
svn rename empty file (465 bytes, patch)
2018-05-26 09:36 PDT, David Kilzer (:ddkilzer)
no flags Details | Formatted Diff | Diff
git rename empty file (265 bytes, patch)
2018-05-26 09:36 PDT, David Kilzer (:ddkilzer)
no flags Details | Formatted Diff | Diff
Patch v1 (6.26 KB, patch)
2018-05-26 21:52 PDT, David Kilzer (:ddkilzer)
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Eric Carlson 2009-09-23 08:12:29 PDT
As noted in https://bugs.webkit.org/show_bug.cgi?id=29623, svn-apply fails when a patch has an entry for a new, empty file.
Comment 1 David Kilzer (:ddkilzer) 2010-07-08 10:49:23 PDT
*** Bug 38801 has been marked as a duplicate of this bug. ***
Comment 2 Daniel Bates 2011-05-10 16:53:50 PDT
svn-apply failed to apply <https://bugs.webkit.org/attachment.cgi?id=92978> (bug #59930).

Notice this patch removes the empty file, LayoutTests/fast/dom/resources/articles.m4a.
Comment 3 David Kilzer (:ddkilzer) 2018-05-26 08:24:41 PDT
The patch from Eric Carlson in Bug 29623 is an svn patch:

Index: LayoutTests/media/content/empty.oga
===================================================================
Index: LayoutTests/media/content/garbage.ogv
===================================================================
--- LayoutTests/media/content/garbage.ogv	(revision 0)
+++ LayoutTests/media/content/garbage.ogv	(revision 0)
@@ -0,0 +1 @@
+moov This is garbage!

And the patch from Daniel Bates in Bug 59930 is a git patch:

diff --git a/LayoutTests/fast/dom/resources/articles.m4a b/LayoutTests/fast/dom/resources/articles.m4a
deleted file mode 100644
index e69de29bb2d1d6434b8b29ae775ad8c2e48c5391..0000000000000000000000000000000000000000
diff --git a/LayoutTests/platform/chromium/test_expectations.txt b/LayoutTests/platform/chromium/test_expectations.txt
index f290aa5db48fed46100d6a042b543f36c6ac307e..4862e52a7d562dd43dc3be049343d679ce274fe9 100644
--- a/LayoutTests/platform/chromium/test_expectations.txt
+++ b/LayoutTests/platform/chromium/test_expectations.txt
@@ -1101,7 +1101,6 @@ BUGCR10350 LINUX : fast/replaced/width100percent-radio.html = FAIL
 
 // These tests are marked as failing on Linux because they depend on QuickTime.
 // See the bug for more discussion.
-BUGCR15797 LINUX : fast/dom/object-embed-plugin-scripting.html = FAIL
 BUGCR15797 LINUX : plugins/embed-attributes-setting.html = FAIL CRASH
 
 // Skip because we throw an exception before we get to dumpAsText.

I think later versions of svn have probably changed to call out the deleted file, so I'm going to use this to fix the git variant.  If the svn variant still happens, we can file a new bug.
Comment 4 David Kilzer (:ddkilzer) 2018-05-26 08:26:03 PDT
Also, it seems like the svn issue happens when adding an empty file, but the git issue happens when deleting an empty file, which are two different bugs!
Comment 5 David Kilzer (:ddkilzer) 2018-05-26 09:16:28 PDT
Creating some experimental patches:

$ svn --version
svn, version 1.9.7 (r1800392)
   compiled Jan 17 2018, 17:16:44 on x86_64-apple-darwin16.1.0

$ git --version
git version 2.15.1 (Apple Git-101)

1. svn add empty file

$ touch LayoutTests/empty
Davids-MacBook-Pro-9:WebKit.svn ddkilzer$ svn add LayoutTests/empty
A         LayoutTests/empty
$ svn diff LayoutTests
Index: LayoutTests/empty
===================================================================
$ ./Tools/Scripts/svn-create-patch LayoutTests
Index: LayoutTests/empty
===================================================================
$ svn --force rm LayoutTests/empty
D         LayoutTests/empty

2. svn delete empty file

$ cat Source/ThirdParty/libwebrtc/WebKit/patch-usrsctp
$ svn rm Source/ThirdParty/libwebrtc/WebKit/patch-usrsctp
D         Source/ThirdParty/libwebrtc/WebKit/patch-usrsctp
$ svn diff Source/ThirdParty/libwebrtc/WebKit
$ ./Tools/Scripts/svn-create-patch Source/ThirdParty/libwebrtc/WebKit
Index: Source/ThirdParty/libwebrtc/WebKit/patch-usrsctp
===================================================================
$ svn revert Source/ThirdParty/libwebrtc/WebKit/patch-usrsctp
Reverted 'Source/ThirdParty/libwebrtc/WebKit/patch-usrsctp'

3. git add empty file

$ touch LayoutTests/empty
$ git add LayoutTests/empty
$ git diff --cached
diff --git a/LayoutTests/empty b/LayoutTests/empty
new file mode 100644
index 00000000000..e69de29bb2d
$ git rm -f LayoutTests/empty
rm 'LayoutTests/empty'

4. git delete empty file

$ git rm -f Source/ThirdParty/libwebrtc/WebKit/patch-usrsctp
rm 'Source/ThirdParty/libwebrtc/WebKit/patch-usrsctp'
$ git diff --cached Source/ThirdParty/libwebrtc/WebKit
diff --git a/Source/ThirdParty/libwebrtc/WebKit/patch-usrsctp b/Source/ThirdParty/libwebrtc/WebKit/patch-usrsctp
deleted file mode 100644
index e69de29bb2d..00000000000
$ git checkout HEAD Source/ThirdParty/libwebrtc/WebKit/patch-usrsctp
Comment 6 David Kilzer (:ddkilzer) 2018-05-26 09:16:57 PDT
Created attachment 341399 [details]
svn add empty file
Comment 7 David Kilzer (:ddkilzer) 2018-05-26 09:17:23 PDT
Created attachment 341400 [details]
svn delete empty file
Comment 8 David Kilzer (:ddkilzer) 2018-05-26 09:18:00 PDT
Created attachment 341401 [details]
git add empty file
Comment 9 David Kilzer (:ddkilzer) 2018-05-26 09:18:34 PDT
Created attachment 341402 [details]
git delete empty file
Comment 10 Build Bot 2018-05-26 09:20:25 PDT
Attachment 341401 [details] did not pass style-queue:


Total errors found: 0 in 0 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 11 David Kilzer (:ddkilzer) 2018-05-26 09:23:21 PDT
Ironically, patches for adding and deleting an empty file using the svn-create-patch tool are identical, so it depends on the state of the repository (whether the file already exists or not) to decide what to do.  (Note that using `svn diff` simply produces no output when deleting a empty file, though.)

The patches for git give a "new file" or "deleted file" hint, so they should never be ambiguous.
Comment 12 David Kilzer (:ddkilzer) 2018-05-26 09:24:19 PDT
(In reply to Build Bot from comment #10)
> Attachment 341401 [details] did not pass style-queue:
> 
> 
> Total errors found: 0 in 0 files
> 
> 
> If any of these errors are false positives, please file a bug against
> check-webkit-style.

And we found a bug in the style-queue as well when you post a patch that just adds an empty file.  :)
Comment 13 David Kilzer (:ddkilzer) 2018-05-26 09:36:07 PDT
Created attachment 341403 [details]
svn rename empty file
Comment 14 David Kilzer (:ddkilzer) 2018-05-26 09:36:37 PDT
Created attachment 341404 [details]
git rename empty file
Comment 15 David Kilzer (:ddkilzer) 2018-05-26 21:35:46 PDT
Comment on attachment 341401 [details]
git add empty file

The success of applying this patch on the EWS bots is a false-positive.  The patch isn't actually added to the repository; it's ignored!
Comment 16 David Kilzer (:ddkilzer) 2018-05-26 21:52:08 PDT
Created attachment 341417 [details]
Patch v1
Comment 17 David Kilzer (:ddkilzer) 2018-05-26 21:53:22 PDT
(In reply to David Kilzer (:ddkilzer) from comment #16)
> Created attachment 341417 [details]
> Patch v1

The irony here is that the empty "resources/empty.txt" file will hit the "false positive" git-add-empty-file bug until this patch lands, so it will need to be landed manually.
Comment 18 David Kilzer (:ddkilzer) 2018-05-26 21:56:24 PDT
Comment on attachment 341417 [details]
Patch v1

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

> Tools/ChangeLog:14
> +        Prior to this change, applying the following patches resulted in:
> +        - svn: add empty file     (failure)
> +        - svn: delete empty file  (failure)
> +        - svn: rename empty file  (failure)
> +        - git: add empty file     (false-positive success)
> +        - git: delete empty file  (success)
> +        - git: rename empty file  (failure)

I should note that the "rename empty file" cases started working once the "delete empty file" cases were fixed because a "rename" is the same as a "copy" followed by a "delete", and copying an empty file worked just fine.
Comment 19 David Kilzer (:ddkilzer) 2018-05-27 09:51:13 PDT
Committed r232228: <https://trac.webkit.org/changeset/232228>
Comment 20 Radar WebKit Bug Importer 2018-05-27 09:52:17 PDT
<rdar://problem/40586646>