Add html5lib to the list of modules in webkitpy/thirdparty/wpt/wpt/tools.

Add html5lib to the list of modules in webkitpy/thirdparty/wpt/wpt/tools.

The purpose of this copy of html5lib is to make the
web-platform-tests `manifest` script work properly - I don't
think I would want to make any other code depend on a library in
//third_party/WebKit/Tools/Scripts/webkitpy/thirdparty/wpt/wpt/tools/.

The reason why I wanted to check in the `manifest` script and all
dependencies here is just so that we can use a pinned version of
the manifest command, so that upstream changes in that script can't
unexpectedly break us (like they did a couple weeks ago).

BUG=681736
Review-Url: https://codereview.chromium.org/2635033005
Cr-Commit-Position: refs/heads/master@{#445771}
Committed: https://chromium.googlesource.com/chromium/src/+/1fc58f0e3b68581b5832176bfd922bc28ecb7b54

[qyearsley, 2017-01-17 19:08:28]

qyearsley@chromium.org changed reviewers:
+ thakis@chromium.org, tkent@chromium.org

[qyearsley, 2017-01-17 19:08:30]

thakis@: Does the change in WebKit/PRESUBMIT.py look OK?

tkent@: Does everything else look OK, including the change in checkout.sh and
WPTWhitelist/WPTHeads?

https://codereview.chromium.org/2635033005/diff/1/third_party/WebKit/PRESUBMI...
File third_party/WebKit/PRESUBMIT.py (right):

https://codereview.chromium.org/2635033005/diff/1/third_party/WebKit/PRESUBMI...
third_party/WebKit/PRESUBMIT.py:21:
r'^third_party[\\\/]WebKit[\\\/]Tools[\\\/]Scripts[\\\/]webkitpy[\\\/]thirdparty[\\\/]wpt[\\\/]wpt[\\\/].*',
There is one file in here (wpt/tools/html5lib/ihatexml.py) that has lines longer
than 800 characters.

Possible solutions:
 - modify this file locally on update
 - exclude just this particular file
 - exclude the directory of files that are imported via script (the above
version)

[tkent, 2017-01-17 22:58:22]

lgtm.  I think the PRESUBMIT.py change is reasonable.

[qyearsley, 2017-01-17 22:59:29]

The CQ bit was checked by qyearsley@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-17 23:00:14]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[qyearsley, 2017-01-17 23:00:57]

qyearsley@chromium.org changed reviewers:
+ dpranke@chromium.org
- thakis@chromium.org

[qyearsley, 2017-01-17 23:02:05]

On 2017/01/17 at 22:58:22, tkent wrote:
> lgtm.  I think the PRESUBMIT.py change is reasonable.

Just realized thakis@ is travelling; Dirk, if you have time could do an OWNER
review of the change to PRESUBMIT.py here?

[commit-bot: I haz the power, 2017-01-18 01:11:56]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-01-18 01:11:57]

Dry run: This issue passed the CQ dry run.

[Dirk Pranke, 2017-01-19 02:37:44]

We already have BeautifulSoup for parsing HTML; can we switch to using html5lib
instead so that we don't have both dependencies?

Also, please update webkitpy/thirdparty/README.chromium with instructions on
where this directory comes from and what we need to do to update it.

And, see
https://chromium.googlesource.com/chromium/src.git/+/master/docs/adding_to_th...
; you need approval from open-source-third-party-reviews@google.com and
security@chromium.org as well.

[qyearsley, 2017-01-19 17:39:47]

On 2017/01/19 at 02:37:44, dpranke wrote:
> We already have BeautifulSoup for parsing HTML; can we switch to using
html5lib instead so that we don't have both dependencies?
> 
> Also, please update webkitpy/thirdparty/README.chromium with instructions on
where this directory comes from and what we need to do to update it.
> 
> And, see
https://chromium.googlesource.com/chromium/src.git/+/master/docs/adding_to_th...
; you need approval from open-source-third-party-reviews@google.com and
security@chromium.org as well.

The purpose of this copy of html5lib is to make the web-platform-tests
`manifest` script work properly - I don't think I would want to make any other
code depend on a library in
//third_party/WebKit/Tools/Scripts/webkitpy/thirdparty/wpt/wpt/tools/.

The reason why I wanted to check in the `manifest` script and all dependencies
here is just so that we can use a pinned version of the manifest command, so
that upstream changes in that script can't unexpectedly break us (like they did
a couple weeks ago). But maybe checking in a copy isn't the best way to do this?
When importing, we're checking out a copy of the web-platform-tests repo, maybe
we can checkout a particular past commit of web-platform-tests when running the
manifest command?

Does that sound like a better idea?

[qyearsley, 2017-01-19 19:09:36]

On 2017/01/19 at 17:39:47, qyearsley wrote:
> On 2017/01/19 at 02:37:44, dpranke wrote:
> > We already have BeautifulSoup for parsing HTML; can we switch to using
html5lib instead so that we don't have both dependencies?
> > 
> > Also, please update webkitpy/thirdparty/README.chromium with instructions on
where this directory comes from and what we need to do to update it.
> > 
> > And, see
https://chromium.googlesource.com/chromium/src.git/+/master/docs/adding_to_th...
; you need approval from open-source-third-party-reviews@google.com and
security@chromium.org as well.
> 
> The purpose of this copy of html5lib is to make the web-platform-tests
`manifest` script work properly - I don't think I would want to make any other
code depend on a library in
//third_party/WebKit/Tools/Scripts/webkitpy/thirdparty/wpt/wpt/tools/.
> 
> The reason why I wanted to check in the `manifest` script and all dependencies
here is just so that we can use a pinned version of the manifest command, so
that upstream changes in that script can't unexpectedly break us (like they did
a couple weeks ago). But maybe checking in a copy isn't the best way to do this?
When importing, we're checking out a copy of the web-platform-tests repo, maybe
we can checkout a particular past commit of web-platform-tests when running the
manifest command?
> 
> Does that sound like a better idea?


Actually, right now we're depending on MANIFEST.json to find WPT tests, so
whenever any new tests are added the manifest script needs to be run. In
http://crbug.com/666957 I was wondering whether we might want to run the
manifest script whenever WPT tests are run, to ensure that the set of tests
found is accurate (although this adds some overhead). If we want to do this,
then I think we *do* want to have a copy of the manifest script checked in,
along with all of its dependencies.

[qyearsley, 2017-01-20 00:48:13]

Description was changed from

==========
Add html5lib to the list of modules in webkitpy/thirdparty/wpt/wpt/tools.

BUG=681736
==========

to

==========
Add html5lib to the list of modules in webkitpy/thirdparty/wpt/wpt/tools.

The purpose of this copy of html5lib is to make the
web-platform-tests `manifest` script work properly - I don't
think I would want to make any other code depend on a library in
//third_party/WebKit/Tools/Scripts/webkitpy/thirdparty/wpt/wpt/tools/.

The reason why I wanted to check in the `manifest` script and all
dependencies here is just so that we can use a pinned version of
the manifest command, so that upstream changes in that script can't
unexpectedly break us (like they did a couple weeks ago).

BUG=681736
==========

[qyearsley, 2017-01-20 00:55:43]

Now updated webkitpy/thirdparty/README.chromium and CL description.

[Dirk Pranke, 2017-01-20 02:24:09]

Checking in the library is the right thing to do (assuming we get approval to do
so, of course :).

[qyearsley, 2017-01-24 17:09:44]

The CQ bit was checked by qyearsley@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-24 17:09:57]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-24 17:14:54]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-01-24 17:14:55]

Dry run: Try jobs failed on following builders:
  cast_shell_android on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/cast_shell_a...)
  chromeos_daisy_chromium_compile_only_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_daisy_...)

[qyearsley, 2017-01-24 18:56:00]

The CQ bit was checked by qyearsley@chromium.org

[qyearsley, 2017-01-24 18:56:02]

The patchset sent to the CQ was uploaded after l-g-t-m from tkent@chromium.org
Link to the patchset: https://codereview.chromium.org/2635033005/#ps20001
(title: "Update webkitpy/thirdparty/README.chromium.")

[commit-bot: I haz the power, 2017-01-24 18:56:50]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-24 19:04:42]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1485284160544380,
"parent_rev": "400c19707cf0ea9dd88a4905833999041dd30214", "commit_rev":
"1fc58f0e3b68581b5832176bfd922bc28ecb7b54"}

[commit-bot: I haz the power, 2017-01-24 19:05:17]

Description was changed from

==========
Add html5lib to the list of modules in webkitpy/thirdparty/wpt/wpt/tools.

The purpose of this copy of html5lib is to make the
web-platform-tests `manifest` script work properly - I don't
think I would want to make any other code depend on a library in
//third_party/WebKit/Tools/Scripts/webkitpy/thirdparty/wpt/wpt/tools/.

The reason why I wanted to check in the `manifest` script and all
dependencies here is just so that we can use a pinned version of
the manifest command, so that upstream changes in that script can't
unexpectedly break us (like they did a couple weeks ago).

BUG=681736
==========

to

==========
Add html5lib to the list of modules in webkitpy/thirdparty/wpt/wpt/tools.

The purpose of this copy of html5lib is to make the
web-platform-tests `manifest` script work properly - I don't
think I would want to make any other code depend on a library in
//third_party/WebKit/Tools/Scripts/webkitpy/thirdparty/wpt/wpt/tools/.

The reason why I wanted to check in the `manifest` script and all
dependencies here is just so that we can use a pinned version of
the manifest command, so that upstream changes in that script can't
unexpectedly break us (like they did a couple weeks ago).

BUG=681736

Review-Url: https://codereview.chromium.org/2635033005
Cr-Commit-Position: refs/heads/master@{#445771}
Committed:
https://chromium.googlesource.com/chromium/src/+/1fc58f0e3b68581b5832176bfd92...
==========

[commit-bot: I haz the power, 2017-01-24 19:05:18]

Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/chromium/src/+/1fc58f0e3b68581b5832176bfd92...