Bug 218105

<rdar://problem/70593576>

Created attachment 412148 [details]
Patch

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

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

Created attachment 412188 [details]
Patch

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

Created attachment 412192 [details]
Patch

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.

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.

Created attachment 412196 [details]
Patch for landing

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

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