[WPT Export] Provisional PR flow does not support binary files |
|||
Issue descriptionVia https://chromium-review.googlesource.com/c/571965 The above CL added a few webm files which caused a patch failure because we get provisional PR patches from the Gerrit API which don't contain binary contents. The PR was made when the CL landed but it ended up being unstable and needed to be reverted. To solve this will take a bit of work, but I think the resolution would be to change the provisional PR flow to fetch and check out the actual Chromium git branch of that CL instead of using the patch from the Gerrit API.
,
Aug 17 2017
I believe the exporter will simply throw an uncaught exception when trying to apply the empty binary patch (from "git apply"). No provisional PRs will be created. When the CL lands, it should be exported correctly.
,
Sep 7 2017
Trying to `git checkout` the CL branch and use `git format-patch` to get the patch instead of from Gerrit HTTP API.
,
Sep 7 2017
`git checkout` would potentially change webkitpy while the scripts are running. Though I believe it would not affect what is already running (scripts are already compiled and loaded into memory), it sounds dangerous. Perhaps fetch is enough and we don't need to actually checkout: FETCH_HEAD might be useful. P.S. if this approach works, issue 759817 should also be fixed in the meantime.
,
Sep 8 2017
Good news! I got a working prototype! And by switching to "git fetch" we actually simplify the code quite a bit. Draft: https://chromium-review.googlesource.com/c/chromium/src/+/658284
,
Sep 15 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/2df6d6918634e0ad0f7b70db39b09363105eb068 commit 2df6d6918634e0ad0f7b70db39b09363105eb068 Author: Robert Ma <robertma@chromium.org> Date: Fri Sep 15 15:09:19 2017 [WPT Export] Get patches of in-flight CLs from their git branches This change fetches the git branch of a CL and uses `git format-patch` to get the patch, instead of using Gerrit API which does not support binary changes. With this change in the workflow, we can also consolidate the processes for both in-flight CLs and Chromium commits, which greatly simplifies the code. Related testing is also improved along the way to use more mocks and avoid unnecessary end-to-end testing. Bug: 743153 , 759817 Change-Id: I069b06e78f30089a31527974f0751032a822bd70 Reviewed-on: https://chromium-review.googlesource.com/658284 Commit-Queue: Robert Ma <robertma@chromium.org> Reviewed-by: Quinten Yearsley <qyearsley@chromium.org> Cr-Commit-Position: refs/heads/master@{#502258} [modify] https://crrev.com/2df6d6918634e0ad0f7b70db39b09363105eb068/third_party/WebKit/Tools/Scripts/webkitpy/common/checkout/git_mock.py [modify] https://crrev.com/2df6d6918634e0ad0f7b70db39b09363105eb068/third_party/WebKit/Tools/Scripts/webkitpy/w3c/gerrit.py [modify] https://crrev.com/2df6d6918634e0ad0f7b70db39b09363105eb068/third_party/WebKit/Tools/Scripts/webkitpy/w3c/gerrit_mock.py [modify] https://crrev.com/2df6d6918634e0ad0f7b70db39b09363105eb068/third_party/WebKit/Tools/Scripts/webkitpy/w3c/gerrit_unittest.py [delete] https://crrev.com/313d77a53f2eafbc69ae61c2612491a8b88b4755/third_party/WebKit/Tools/Scripts/webkitpy/w3c/resources/expected.patch [delete] https://crrev.com/313d77a53f2eafbc69ae61c2612491a8b88b4755/third_party/WebKit/Tools/Scripts/webkitpy/w3c/resources/expected2.patch [delete] https://crrev.com/313d77a53f2eafbc69ae61c2612491a8b88b4755/third_party/WebKit/Tools/Scripts/webkitpy/w3c/resources/sample.patch [delete] https://crrev.com/313d77a53f2eafbc69ae61c2612491a8b88b4755/third_party/WebKit/Tools/Scripts/webkitpy/w3c/resources/sample2.patch [modify] https://crrev.com/2df6d6918634e0ad0f7b70db39b09363105eb068/third_party/WebKit/Tools/Scripts/webkitpy/w3c/test_exporter.py [modify] https://crrev.com/2df6d6918634e0ad0f7b70db39b09363105eb068/third_party/WebKit/Tools/Scripts/webkitpy/w3c/test_exporter_unittest.py
,
Sep 15 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/60c683c1d66d9ba50220845150cc27caeedf3b7a commit 60c683c1d66d9ba50220845150cc27caeedf3b7a Author: Robert Ma <robertma@chromium.org> Date: Fri Sep 15 23:00:04 2017 WPT export: Add code review links back to provisional PRs crrev.com/c/658284 switched to using git fetch for in-flight CLs, which unintentionally changed what was included in PR description. The commit message (of an in-flight CL) does not contain the Reviewed-on footer (the link to code review), which is useful and added back in this CL. Besides, in this CL I re-examined the ending newlines in commit messages and PR description to make sure the mock is consistent with real output and there is no unintended empty new line. Bug: 743153 Change-Id: If0f820d70bb47c0c27427d56bdbef590c71d759a Reviewed-on: https://chromium-review.googlesource.com/669008 Reviewed-by: Quinten Yearsley <qyearsley@chromium.org> Commit-Queue: Robert Ma <robertma@chromium.org> Cr-Commit-Position: refs/heads/master@{#502420} [modify] https://crrev.com/60c683c1d66d9ba50220845150cc27caeedf3b7a/third_party/WebKit/Tools/Scripts/webkitpy/w3c/chromium_commit_mock.py [modify] https://crrev.com/60c683c1d66d9ba50220845150cc27caeedf3b7a/third_party/WebKit/Tools/Scripts/webkitpy/w3c/gerrit.py [modify] https://crrev.com/60c683c1d66d9ba50220845150cc27caeedf3b7a/third_party/WebKit/Tools/Scripts/webkitpy/w3c/gerrit_unittest.py [modify] https://crrev.com/60c683c1d66d9ba50220845150cc27caeedf3b7a/third_party/WebKit/Tools/Scripts/webkitpy/w3c/test_exporter.py [modify] https://crrev.com/60c683c1d66d9ba50220845150cc27caeedf3b7a/third_party/WebKit/Tools/Scripts/webkitpy/w3c/test_exporter_unittest.py
,
Sep 18 2017
|
|||
►
Sign in to add a comment |
|||
Comment 1 by foolip@chromium.org
, Aug 17 2017