search

mozilla-services / syncserver

Run-Your-Own Firefox Sync Server
Mozilla Public License 2.0
1.87k stars 145 forks source link

makefile: use the system Python's virtualenv #148

Closed mathstuf closed 6 years ago

mathstuf commented 6 years ago

virtualenv on Fedora is now a Python3 tool. Without installing Python3, there is no virtualenv tool. Instead, just rely on the package being installed.

mathstuf commented 6 years ago

Hrm. Seems that breaks CI :/ . How does the Python have a virtualenv, but no python -m virtualenv?

okin commented 6 years ago

The module in the stdlib is called venv. I don't know if presenting the module as virtualenv is a Fedora thing or you might have the external version of this module installed. You can probalby find this out with something like rpm -qa | grep virtualenv.

venv is available since Python 3.3. Unfortunately syncserver currently only supports legacy Python. I think it would be bad to just assume there will be Python 3 available everywhere - especially if things are run in any sort of container.

mathstuf commented 6 years ago

That's the case in Python3, but I'm still running it with Python2 (since that's the only supported version anyways).

okin commented 6 years ago

Sorry, probably got carried away with the Python 3 way. Tested it on my Debian box and the change works as desired.

For the Travis builds I think the problem we are running into is that they are starting out in an virtual environment and there seems to be no virtualenv available inside. Adding it to the install sections for Travis seems to fix the problem - see my test commit on top of your change (test results).

rfk commented 6 years ago

Thanks @mathstuf, this change looks reasonable to me, please try including @okin's fix for CI and I'll be happy to merge.

rfk commented 6 years ago

Thanks @mathstuf, this change looks reasonable to me, please try including @okin's fix for CI and I'll be happy to merge.

rfk commented 6 years ago

Thanks @mathstuf, this change looks reasonable to me, please try including @okin's fix for CI and I'll be happy to merge.

mathstuf commented 6 years ago

I cherry-picked @okin's commit and touched up the commit message a bit.