Bug 29684 – svn-apply fails when a patch has an empty file

• Status: RESOLVED
• Resolution: FIXED
• Component: Tools / Tests

[Eric Carlson]

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.

[David Kilzer (:ddkilzer)]

*** Bug 38801 has been marked as a duplicate of this bug. ***

[Daniel Bates]

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.

[David Kilzer (:ddkilzer)]

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.

[David Kilzer (:ddkilzer)]

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!

[David Kilzer (:ddkilzer)]

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

[David Kilzer (:ddkilzer)]

Created attachment 341399 [details]
svn add empty file

[David Kilzer (:ddkilzer)]

Created attachment 341400 [details]
svn delete empty file

[David Kilzer (:ddkilzer)]

Created attachment 341401 [details]
git add empty file

[David Kilzer (:ddkilzer)]

Created attachment 341402 [details]
git delete empty file

[EWS Watchlist]

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.

[David Kilzer (:ddkilzer)]

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.

[David Kilzer (:ddkilzer)]

(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.  :)

[David Kilzer (:ddkilzer)]

Created attachment 341403 [details]
svn rename empty file

[David Kilzer (:ddkilzer)]

Created attachment 341404 [details]
git rename empty file

[David Kilzer (:ddkilzer)]

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!

[David Kilzer (:ddkilzer)]

Created attachment 341417 [details]
Patch v1

[David Kilzer (:ddkilzer)]

(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.

[David Kilzer (:ddkilzer)]

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.

[David Kilzer (:ddkilzer)]

Committed r232228: <https://trac.webkit.org/changeset/232228>

[Radar WebKit Bug Importer]

<rdar://problem/40586646>