New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 691107 link

Starred by 2 users

Issue metadata

Status: WontFix
Owner: ----
Closed: Jul 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

[WPT auto-import] Patch failure occurs when importing lines with non-Unix line endings

Project Member Reported by qyears...@chromium.org, Feb 10 2017

Issue description

Specifically, this file has caused a recent auto-import to fail:
webvtt/webvtt-file-format-parsing/webvtt-file-parsing/support/newlines.vtt

Log:
https://build.chromium.org/p/tryserver.blink/builders/mac10.11_blink_rel/builds/1941/steps/bot_update/logs/patch%20error

Log excerpt:
Failed to apply patch for third_party/WebKit/LayoutTests/external/wpt/webvtt/webvtt-file-format-parsing/webvtt-file-parsing/support/newlines.vtt:
While running git apply --index -3 -p1 --verbose;
  fatal: corrupt patch at line 10

Patch:   N   third_party/WebKit/LayoutTests/external/wpt/webvtt/webvtt-file-format-parsing/webvtt-file-parsing/support/newlines.vtt
Index: third_party/WebKit/LayoutTests/external/wpt/webvtt/webvtt-file-format-parsing/webvtt-file-parsing/support/newlines.vtt
diff --git a/third_party/WebKit/LayoutTests/external/wpt/webvtt/webvtt-file-format-parsing/webvtt-file-parsing/support/newlines.vtt b/third_party/WebKit/LayoutTests/external/wpt/webvtt/webvtt-file-format-parsing/webvtt-file-parsing/support/newlines.vtt
new file mode 100644
index 0000000000000000000000000000000000000000..ee2f00804a7e9556f3c7bce5514491fd796512d5
--- /dev/null
+++ b/third_party/WebKit/LayoutTests/external/wpt/webvtt/webvtt-file-format-parsing/webvtt-file-parsing/support/newlines.vtt
@@ -0,0 +1,12 @@
+WEBVTT

cr
00:00:00.000 --> 00:00:01.000
{"id":"cr"}
+
+lf
+00:00:00.000 --> 00:00:01.000
+{"id":"lf"}
+
+crlf
+00:00:00.000 --> 00:00:01.000
+{"id":"crlf"}
+
lfcr
00:00:00.000 --> 00:00:01.000
+{"id":"lfcr"}
+
\ No newline at end of file
 

Comment 1 by foolip@chromium.org, Feb 10 2017

Good times. Does this mean that there are a lot of HTML parser tests which are also failing? Is it safe to assume that this problem would go away with Gerrit, which is based on Git instead of patch files?
It depends on whether the problem is with the patch storage in Rietveld, or the patch application in the bot_update step. Whichever it is, it's probably also the same problem that's causing  bug 656171 .

Possible next steps to investigate this:
 1. Try to make test CLs on both Rietveld and Gerrit with various line endings, and triggering try jobs. If patch failures happen on both Rietveld and Gerrit, this suggests that the failure might be related to patch application.
 2. If the failure is related to patch storage/application that's not specific to Rietveld, then maybe the next place to look is build/scripts/slave/bot_update.py or depot_tools/apply_issue.py.

Meanwhile, until this is solved, the temporary workaround is to skip some tests :-/
Project Member

Comment 3 by bugdroid1@chromium.org, Feb 10 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/8b137c363843aa9d1bb7783bb364a0584e238d6f

commit 8b137c363843aa9d1bb7783bb364a0584e238d6f
Author: qyearsley <qyearsley@chromium.org>
Date: Fri Feb 10 23:41:57 2017

W3CImportExpectations: Skip webvtt/webvtt-file-format-parsing/webvtt-file-parsing/001.html.

R=maksim.sisov@intel.com
BUG= 691107 

Review-Url: https://codereview.chromium.org/2690633004
Cr-Commit-Position: refs/heads/master@{#449789}

[modify] https://crrev.com/8b137c363843aa9d1bb7783bb364a0584e238d6f/third_party/WebKit/LayoutTests/W3CImportExpectations

Comment 4 by tkent@chromium.org, Feb 12 2017

Cc: tkent@chromium.org
Is this resolved if we have .gitattributes to indicate the file is not text?


I think it would -- good idea :-D

In other particular cases where there needs to be various different line endings in the test file (anything with CR AT EOL in https://github.com/w3c/web-platform-tests/blob/master/lint.whitelist), that may also work.

Do you think it would make more sense to put this .gitattributes file upstream, or in Chromium (e.g. in LayoutTests/external/)?

Comment 6 by tkent@chromium.org, Feb 14 2017

I thought .gitattributes in the upstream.
However, WPT importer should be robust, and detecting CR line endings and adding .gitattributes automatically would be another option.



Note: if this isn't solved after migrating to Gerrit ( bug 693792 ), adding entries in .gitattributes would be a good next thing to try.
Cc: -maksim.s...@intel.com maksim.sisov@chromium.org
Components: Blink>Infra>Ecosystem
Components: -Blink>Infra>Predictability
Status: WontFix (was: Available)
Haven't seen this in a while, not sure if it's still an issue, closing bug for now.

Sign in to add a comment