Bug 197046 – AX: Improve the exception message in Tools/Scripts/webkitpy/common/system/autoinstall.py (line 360)

• Component: Tools / Tests
• Status: RESOLVED INVALID

[yevseytsev]

When I was preparing to my first contributions to WebKit I had some issues with running "check-webkit-style" script. I was getting the exception coming from Tools/Scripts/webkitpy/common/system/autoinstall.py (line 360). Exception message:

"Could not download Python modules from URL "None".
Make sure you are connected to the internet.
You must be connected to the internet when
downloading needed modules for the first time."

It was trying to install the pep8 module but was failing because it did not know what URL to download it from and said that I was not connected to the internet. But actually, I was connected to the Internet and had a stable connection. Then I reinstalled Python and WebKit developer tools, but nothing helped. After that, I reached out to the WebKit community on IRC and we began to look where the problem can be. The first thing that came to mind is blocking of TLS < 1.2 by pypi (files.pythonhosted.org) and some contributors suggested me to reinstall Python manually, but it did not help as well. Then I reached out to other contributors who helped me before with other bugs concerning building WebKit and they did not know as well what can cause it, but helped me with pinging Jonathan Bedard (from Tools/Operations team, who owns most of the code in Tools/Scripts).

Jonathan reached me out on IRC webkit channel and we began to try to find where the problem can be. Firstly he suggested me to delete Tools/Scripts/webkitpy/thirdparty/autoinstalled and run the script again to see what is going to happen, the folder appeared again and the same error was shown. Jonathan told that he saw this error on High Sierra machines, but never Mojave(!!!). After we continued to look where the error can be and Jonathan came up with idea that probably I have an old version of urllib2 ssl libraries, and I uninstalled them and installed again, but it did not help as well. We tried reinstalling multiple libraries and were wondering which library actually is responsible for my error. After trying all the libraries we could imagine the error still existed.

After some time Jonathan came up with small repro case:

import urllib2
url = 'https://files.pythonhosted.org/packages/source/p/pep8/pep8-0.5.0.tar.gz'
urllib2.urlopen(url, timeout=30)

I ran it on my machine, while Jonathan ran it on lab machines in Apple and surprisingly we got different errors:

Lab machines got this: <urlopen error [SSL: TLSV1_ALERT_PROTOCOL_VERSION] tlsv1 alert 
protocol version (_ssl.c:590)>
I got this: <urlopen error [SSL: CERTIFICATE_VERIFY_FAILED] certificate verify 
failed (_ssl.c:727)>

That was weird because I had the first one before I reinstalled python manually. 

Then Jonathan came up with the idea that Python on my Mac does not have access to its system certs so it can't verify any of the URLs. After this, I researched some errors again and one of them was very weird, it said:

"The directory '/Users/appletest/Library/Caches/pip' or its parent directory is not owned by the current user and caching wheels has been disabled. check the permissions and owner of that directory. If executing pip with sudo, you may want sudo's -H flag.Collecting urllib2-ext"

It was trying to reach an appletest user on Mac which was the "super admin user", but I deleted it 2 years ago and somehow it was still on my machine. After running "sudo pip install requests" I got a very similar error with appletest user: 

"The directory '/Users/appletest/Library/Caches/pip/http' or its parent directory is not owned by the current user and the cache has been disabled. Please check the permissions and owner of that directory. If executing pip with sudo, you may want sudo's -H flag.The directory '/Users/appletest/Library/Caches/pip' or its parent directory is not owned by the current user and caching wheels has been disabled. check the permissions and owner of that directory. If executing pip with sudo, you may want sudo's -H flag."

Finally, we came up with the conclusion that Python is owned by appletest user (which was deleted, I was running user called - yevseytsev). We wanted to delete it but we saw that my Mac has a weird set up, and I could not even delete the appletest user under users folder or in settings of my Mac, so I decided to completely reset my Mac and reinstalled the system from the very beginning. After this, everything was working good and I was able to run any webkit script on my Mac.

Going back to the exception message that I received on the beginning: 

"Could not download Python modules from URL "None".
Make sure you are connected to the internet.
You must be connected to the internet when
downloading needed modules for the first time"

It can be easily seen that not only the internet connection can cause this problem, it also can be caused by wrong set up of your system, users or Python.

I think the exception message should have a more detailed description and say about other possible causes as well (system/user set up or Python), in order to protect future contributors who can get this error from wrong interpretation what can cause it. 

Please let me know if this will be a reasonable fix to the exception message, I really believe that the message should be improved.

[yevseytsev]

Sorry for the super long description, but I wanted to describe every small part in order to make it clear

[yevseytsev]

Created attachment 367717 [details]
Patch

[EWS Watchlist]

Note that there are important steps to take when updating ANGLE. See http://trac.webkit.org/wiki/UpdatingANGLE

[yevseytsev]

Added a patch with improved exception message "...Additionally, make sure Python is owned by the current user and it can access its system certificates in order to verify the URLs." Please let me know whether it is good or I need to improve/change something.

[Alexey Proskuryakov]

Comment on attachment 367717 [details]
Patch

There is something wrong with this patch, as it's half a megabyte in size, and touches lots of ANGLE files. Perhaps some newline format change made tools think that all files were rewritten?

[yevseytsev]

Created attachment 367739 [details]
Patch

[yevseytsev]

Fixed patch to affect only needed file

[Alexey Proskuryakov]

Comment on attachment 367739 [details]
Patch

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

> Tools/Scripts/webkitpy/common/system/autoinstall.py:386
> +                           "Additionally, make sure Python is owned by the "
> +                           "current user and it can access its system "

I'm not sure what it means for Python to be owned by the current user. /usr/bin/python certainly isn't.

[yevseytsev]

(In reply to Alexey Proskuryakov from comment #8)
> Comment on attachment 367739 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=367739&action=review
> 
> > Tools/Scripts/webkitpy/common/system/autoinstall.py:386
> > +                           "Additionally, make sure Python is owned by the "
> > +                           "current user and it can access its system "
> 
> I'm not sure what it means for Python to be owned by the current user.
> /usr/bin/python certainly isn't.


"Additionally, make sure Python can access its system certificates in order to verify the URLs."
Do you think this message would be better?

[Alexey Proskuryakov]

That doe sound better indeed, although it's not very actionable without more research (how exactly does one verify that?)

Jonathan, you already worked with Andriy on this, would you like to weigh in?

[Dean Johnson]

The error messages being improved here, on their own, would probably have saved me multiple hours of debugging in the past two years of hitting similar issues to this.

Most of the problems around this to me seem to be around downloading, though. I think it would be better for us to move the downloading here from being urllib2-based to curl-based (except for Windows), and to pass the --no-verify flag since we will be checking the hash of the downloaded file against the hash saved in __init__.py anyways.

Created: https://bugs.webkit.org/show_bug.cgi?id=197164

[Jonathan Bedard]

Comment on attachment 367739 [details]
Patch

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

>>> Tools/Scripts/webkitpy/common/system/autoinstall.py:386
>>> +                           "current user and it can access its system "
>> 
>> I'm not sure what it means for Python to be owned by the current user. /usr/bin/python certainly isn't.
> 
> "Additionally, make sure Python can access its system certificates in order to verify the URLs."
> Do you think this message would be better?

I think that the the second one is a better (and more accurate) description of the problem. I would remove 'its', though.

[Dean Johnson]

I can reproduce the TLS version error when running locally if I remove all the autoinstalled packages. Switching to a curl-based workflow instead of urllib2 seems to have gotten me around those. 

PFR: https://bugs.webkit.org/show_bug.cgi?id=197164

[Sam Sneddon [:gsnedders]]

Bug 218131 removed the old autoinstaller, and this isn't meaningful any more.