freedomofpress / securethenews

An automated scanner and web dashboard for tracking TLS deployment across news organizations
https://securethe.news
GNU Affero General Public License v3.0
100 stars 25 forks source link

`make dev` fails when attempting to obtain pshtt #118

Closed joedougherty closed 7 years ago

joedougherty commented 7 years ago

The Ansible playbook fails on the securethenews-deploy : Install pshtt task.

pshtt_failure

I've tried building this using both Docker and Virtualbox as the provider: both produced the same failure. I initially suspected the issue may have been the specific commit the install path linked to (7362a8bdabe7190a933e4c5e18561ba9d07dc561), but I was able to install locally on Ubuntu 16.04.

I connected to the VM via vagrant ssh and ran sudo /usr/local/bin/pip2.7 install -e git+git://github.com/dhs-ncats/pshtt@7362a8bdabe7190a933e4c5e18561ba9d07dc561#egg=pshtt. This resulted in:


Obtaining pshtt from git+git://github.com/dhs-ncats/pshtt@7362a8bdabe7190a933e4c5e18561ba9d07dc561#egg=pshtt
  Skipping because already up-to-date.
Collecting requests>=2.10.0 (from pshtt)
  Using cached requests-2.17.3-py2.py3-none-any.whl
Collecting sslyze>=0.13.6 (from pshtt)
  Using cached SSLyze-1.1.1.zip
    Complete output from command python setup.py egg_info:
    error in SSLyze setup command: Invalid environment marker: python_version < "3.4"

    ----------------------------------------
Command "python setup.py egg_info" failed with error code 1 in /tmp/pip-build-2ClI_P/sslyze/
garrettr commented 7 years ago

The problem is SSLyze (one could argue that this also somewaht pshtt's fault, for not pinning the SSLyze dependency). My first guess would be that this is related to SSLyze's recent addition of Python 3 support: https://github.com/nabla-c0d3/sslyze/pull/203. Specifically, they added environment markers to their setup.py's extras_require, which seems to be poorly supported by some versions of Python and/or setuptools (e.g. https://github.com/docker/docker-py/issues/1019).

In the short term, I might be able to fix this by upgrading setuptools in the development environment before installing the Python requirements.

In the long term, now that SSLyze supports Python 3, we'd like to get pshtt to also support Python 3 (https://github.com/dhs-ncats/pshtt/issues/4). Since Secure the News is already written in Python 3, once SSLyze and pshtt support Python 3 we could drop our annoying requirement to have both a Python 2 and a Python 3 environment for Secure the News.

garrettr commented 7 years ago

Looks like this is a known issue to the SSLyze maintainers, and it's caused by a bug in setuptools. Best fix seems to be to upgrade setuptools.

garrettr commented 7 years ago

I am able to reproduce.

joedougherty commented 7 years ago

@garrettr Thanks for the confirmation.