Bug 218105 – [webkitpy] Use webkitcorepy's autoinstaller for buildbot and twisted

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

[Jonathan Bedard]

The way we handle twisted and Buildbot are related, since buildbot requires specific versions of twisted.

[Radar WebKit Bug Importer]

<rdar://problem/70593576>

[Jonathan Bedard]

Created attachment 412148 [details]
Patch

[Jonathan Bedard]

This is also the last change for https://bugs.webkit.org/show_bug.cgi?id=214950, so it removes all the auto-installer code.

[Aakash Jain]

(In reply to Jonathan Bedard from comment #3)
> This is also the last change for
> https://bugs.webkit.org/show_bug.cgi?id=214950, so it removes all the auto-installer code.
Let’s remove the auto-installer code in separate patch, and keep this patch for migrating buildbot and twisted, so that it is easier to review.

[Jonathan Bedard]

Created attachment 412188 [details]
Patch

[Jonathan Bedard]

(In reply to Aakash Jain from comment #4)
> (In reply to Jonathan Bedard from comment #3)
> > This is also the last change for
> > https://bugs.webkit.org/show_bug.cgi?id=214950, so it removes all the auto-installer code.
> Let’s remove the auto-installer code in separate patch, and keep this patch
> for migrating buildbot and twisted, so that it is easier to review.

Uploaded a change that does that. Still need to remove the unit tests, though.

[Jonathan Bedard]

Created attachment 412192 [details]
Patch

[Aakash Jain]

Comment on attachment 412192 [details]
Patch

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

rs=me

> Tools/Scripts/libraries/webkitcorepy/webkitcorepy/autoinstall.py:157
> +                    if element.childNodes[0].data.endswith('.tar.gz') or element.childNodes[0].data.endswith('.tar.bz2'):

str.endswith also accepts a tuple https://docs.python.org/2/library/stdtypes.html#str.endswith , can rewrite this as:

if element.childNodes[0].data.endswith(('.tar.gz', '.tar.bz2'))

> Tools/Scripts/libraries/webkitcorepy/webkitcorepy/autoinstall.py:273
> +                location = os.path.join(AutoInstall.directory, self.name.split('.')[0])

Please add a comment mentioning the reason to add this logic.

> Tools/Scripts/webkitpy/autoinstalled/buildbot.py:27
> +if sys.version_info[0] > 2:

Maybe make it more explicit that we are referencing python3 here by changing it to: ">= 3"

> Tools/Scripts/webkitpy/autoinstalled/buildbot.py:45
> +    AutoInstall.register(Package('twisted', Version(12, 1, 0), pypi_name='Twisted'))

It's little confusing that we are installing twisted version 12.1 here and 15.5 in twisted.py, but it was probably like this earlier as well.

[Jonathan Bedard]

Comment on attachment 412192 [details]
Patch

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

>> Tools/Scripts/webkitpy/autoinstalled/buildbot.py:45
>> +    AutoInstall.register(Package('twisted', Version(12, 1, 0), pypi_name='Twisted'))
> 
> It's little confusing that we are installing twisted version 12.1 here and 15.5 in twisted.py, but it was probably like this earlier as well.

Agreed. My hope is that moving forward, everything can be version 20.4.1...but we need to qualify performance infrastructure using that, and I'm pretty sure buildbot 0.8 needs a smaller version of twisted. That's the reason that these are in an autoinstalled directory instead of at the top level, by the way, it allows us to exclude certain packages.

[Jonathan Bedard]

Created attachment 412196 [details]
Patch for landing

[EWS]

Committed r268930: <https://trac.webkit.org/changeset/268930>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 412196 [details].